Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deterministic Bundle Trees #309

Closed

Conversation

brandon93s
Copy link
Contributor

Potential fix for #219.

Promise.all guarantees order. This delays the set operations until after processing so that they always occur in the same order.

Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is the best solution performance wise

await this.loadAsset(assetDep);
}
return applyFunction;
})
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is applyFunctions only job to be run later. Then would it be leaner to chain .forEach(fn => fn()) do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would work as well. I felt the current was more readable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good. Confirmed test is stable on my end now.

@pomle
Copy link
Contributor

pomle commented Dec 16, 2017

👍

devongovett added a commit that referenced this pull request Dec 19, 2017
@devongovett
Copy link
Member

Thanks for looking into this! I refactored it slightly to be two loops: one that resolves and loads assets, and the second which assigns them back to deps. 539ade1

@brandon93s
Copy link
Contributor Author

Awesome, definitely a bit cleaner. Hopefully the last we see of this test failing!

@brandon93s brandon93s deleted the deterministic-bundle-tree branch January 28, 2018 19:14
devongovett added a commit that referenced this pull request Oct 15, 2018
devongovett added a commit that referenced this pull request Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants