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

Automated npm/browserify/cjs loader #2505

Merged
merged 2 commits into from
Nov 27, 2014
Merged

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Nov 15, 2014

This closes #2283.

It extends the ember-cli addon API in two ways:

  • I've added a new 'js' type of postprocessTree. This was suggested by @rwjblue and it nicely solves this particular problem, while being generally useful for other addons.
  • addons can use app.importWhitelistFilters to register filters that determine which module names should be ignored by the es6 concatenator.

With these extensions, my new ember-browserify addon can do the actual work of finding & packaging npm modules.

Remaining issues before this is ready to merge:

@@ -652,7 +654,7 @@ EmberApp.prototype.javascript = function() {

var es6 = compileES6(applicationJs, {
loaderFile: 'vendor/ember-cli/loader.js',
ignoredModules: Object.keys(this.importWhitelist),
ignoredModules: this._isIgnoredModule.bind(this),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

An earlier idea I had was to pass ignored Modules by reference, which if preserved would allow updating the pojo to be sufficient. Last I looked, it was possible to fix the concatenator to ensure its binding wasn't broken.
Thoughts? This would also mitigate my later concern (although I do not know its magnitude so it may be worrying for no reason)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mutating shared objects across module boundaries gives me the heeby jeebies.

@stefanpenner
Copy link
Contributor

Awesome, I am excited for this. I left some comments here and on ember-browserify, nothing blocking merely some feedback.

@stefanpenner
Copy link
Contributor

Ya let's avoid the shared state. LGTM :)

@rwjblue
Copy link
Member

rwjblue commented Nov 15, 2014

👍

@stefanpenner
Copy link
Contributor

@ef4 travis appears unhappy

@ef4
Copy link
Contributor Author

ef4 commented Nov 18, 2014

Yeah I think it's because I'm pointing at my es6 concatenator fork still and not a proper version number.

@stefanpenner
Copy link
Contributor

I've opened the appropriate PR on joliss/es6-broccoli-concatenator project joliss/broccoli-es6-concatenator#29

@ef4
Copy link
Contributor Author

ef4 commented Nov 18, 2014

Ok cool, good timing. I just squashed that branch to prepare to making a PR
myself.

On Mon, Nov 17, 2014 at 10:41 PM, Stefan Penner notifications@github.com
wrote:

I've opened the appropriate PR on joliss/es6-broccoli-concatenator project
joliss/broccoli-es6-concatenator#29
joliss/broccoli-es6-concatenator#29


Reply to this email directly or view it on GitHub
#2505 (comment)
.

@stefanpenner
Copy link
Contributor

:)

@stefanpenner stefanpenner added this to the 0.1.3 milestone Nov 18, 2014
@stefanpenner
Copy link
Contributor

@ef4 any ideas re: travis failures?

@ef4
Copy link
Contributor Author

ef4 commented Nov 22, 2014

@stefanpenner , yeah we're waiting on a semver-parsible version of broccoli-es6-concatenator that has the requisite changes. Once one is published I will update my PR to point at it.

1) dependencies in package.json are locked down for pre-1.0 versions:
TypeError: Invalid comparator: git://github.com/ef4/broccoli-es6-concatenator
at Comparator.parse (/home/travis/build/stefanpenner/ember-cli/node_modules/semver/semver.js:605:11)

@stefanpenner stefanpenner modified the milestones: v0.2.0, 0.1.3 Nov 25, 2014
@stefanpenner
Copy link
Contributor

we are still blocked on @joliss review + feedback on the broccoli-es6-concatenator project bumping this to v0.2.0

@jakecraige
Copy link
Member

😢 i want this so baddd

rwjblue added a commit that referenced this pull request Nov 27, 2014
Automated npm/browserify/cjs loader
@rwjblue rwjblue merged commit 4360457 into ember-cli:master Nov 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] dependencies as easy as browserify
4 participants