Skip to content

optionally prevent a double AMD compile#144

Closed
kellyselden wants to merge 2 commits into
ember-cli:masterfrom
kellyselden:preventDoubleAmdCompile
Closed

optionally prevent a double AMD compile#144
kellyselden wants to merge 2 commits into
ember-cli:masterfrom
kellyselden:preventDoubleAmdCompile

Conversation

@kellyselden

Copy link
Copy Markdown
Member

The AmdFunnel in ember-cli has optimizations in it to do as little file IO as possible. https://github.com/ember-cli/ember-cli/blob/bd36eca668f8fab21cc9cdfdaae97ffcdc1de9ee/lib/broccoli/ember-app.js#L1652. As it turns out, these optimizations are not good enough, and early exit doesn't seem possible. I decided to try a different approach, to catch the string right before the compile and prevent it going through a second time. The AmdFunnel code can be removed if this is merged.

cc @twokul

@kellyselden
kellyselden requested a review from rwjblue July 1, 2018 20:05
@twokul

twokul commented Jul 1, 2018

Copy link
Copy Markdown
Contributor

wouldn't we want preventDoubleAmdCompile to always be on?

@kellyselden

kellyselden commented Jul 1, 2018

Copy link
Copy Markdown
Member Author

In ember-cli, yes. I didn't want to make any assumptions about other potential consumers of this lib.

@kellyselden

Copy link
Copy Markdown
Member Author

Added a callback so ember-cli can print the same deprecation warning as it did with AmdFunnel.

@stefanpenner

Copy link
Copy Markdown
Contributor

I believe the work that motivated this change has taken a different direction, closing for now. Let me know if I have closed this in error.

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.

3 participants