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

Move loader.js to be included as an addon. #5379

Merged
merged 1 commit into from Jan 22, 2016

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Jan 20, 2016

This allows us to:

  • Remove another bower dependency
  • Ensure that ember-cli is properly paired with the loader.js version
  • Remove reexporter code, since we now know for certain what loader.js we have.

Requires ember-cli/loader.js#64.

@rwjblue
Copy link
Member Author

rwjblue commented Jan 20, 2016

ember-cli/loader.js#64 was released as v4.0.0, rebasing this PR to include it.

@rwjblue rwjblue force-pushed the use-loader-as-internal-dep branch 2 times, most recently from da49dd0 to 1e50818 Compare January 20, 2016 15:42
@homu
Copy link
Contributor

homu commented Jan 20, 2016

☔ The latest upstream changes (presumably #5367) made this pull request unmergeable. Please resolve the merge conflicts.

@nathanhammond
Copy link
Contributor

I would prefer that the prepend check for this to be special-cased. That way in an ideal scenario it's:

  • loader
  • bunch of modules
  • one invocation to unravel the world

That makes the way I'm thinking about service workers a lot easier.

Note: I don't want to let perfect be the enemy of good, if we're comfortable going back and adding this when necessary, let's ship this as is. (Pretty sure that would be backwards compatible too.)

@rwjblue
Copy link
Member Author

rwjblue commented Jan 22, 2016

@nathanhammond and I discussed this at length in #dev-ember-cli. The current plan of attack is to:

  1. Ensure that app.import('adsfaf', { prepend: true }); maintains the correct order (currently the last thing to be prepended is first, which is a bit of a :trollface:)
  2. Leave loader.js as a user defined addon. This allows other addons that need to prepend (but still depend on the loader) to declare after: 'loader.js' in and they would be loaded at the correct time after the loader.
  3. Add after: ['loader.js'] to default addon blueprint.

I am going to continue this PR and try to get it in a mergeable state (there are a couple addon tests failing ATM), and then follow this PR up with points 1 through 3 from above as a separate PR (that either @nathanhammond or I will work on tonight/tomorrow).

@rwjblue rwjblue force-pushed the use-loader-as-internal-dep branch 2 times, most recently from 242b1ab to 96a0f13 Compare January 22, 2016 05:05
This allows us to:

* Remove another bower dependency
* Ensure that ember-cli is properly paired with the loader.js version
* Remove reexporter code, since we now know for certain what loader.js
  we have.
@stefanpenner
Copy link
Contributor

👍

rwjblue added a commit that referenced this pull request Jan 22, 2016
Move loader.js to be included as an addon.
@rwjblue rwjblue merged commit ac3b7ac into ember-cli:master Jan 22, 2016
@rwjblue rwjblue deleted the use-loader-as-internal-dep branch January 22, 2016 05:53
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.

None yet

5 participants