-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
ES6 modules #68
ES6 modules #68
Conversation
|
||
```js | ||
import Component from 'ember-component'; | ||
import { isBlank } from 'ember-utils'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export property buckets, aren't really super tree-shaking friendly. I don't mind, but its worth considering, ember-utils/is-blank
for those who want to benefit from the simple tree shaking.
It may be nice to keep the buckets, so users who don't care have an easy experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no way to do tree-shaking for named exports or just harder than for defaults?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
significantly more difficult.
It requires much more machinery/complexity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you point to an sample implementation? I'm happy to dig in, since I feel introducing more than one way to access Ember stuff brings only confusion to newcomers especially.
While I agree with this, it would probably be better if ember was a single addon that provided the different built packages. From a maintainability point this is much better what developers add to their project is not much different. Instead of |
@chadhietala - We definitely discussed that at length, but we prefer this modules listing as it allows us to split things up properly in the future without changing our public module API. Right now, NPM doesn't really allow us to split them, but we definitely plan to do so as they add better support... |
Here, both `Ember.Component` and `Ember.isBlank` could be `import`ed: | ||
|
||
```js | ||
import Component from 'ember-component'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be mentioned that Ember's repo will change to reflect this new structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, the shims repo update will allow 1.x apps to start using the new module structure but Ember 2.x will provide these exports and will also maintain this internal structure.
While I prefer the aesthetics of As you noted above, we plan to re-arrange Ember's internal structure to match the proposed module structure in this RFC. We have two options for distributing that: as one big package that we treeshake for unused code, or as a series of smaller packages with an Obviously, the move from Ember CLI apps consuming globals to modules is a fairly significant change for people to make. If we use |
What about using npm's scoped packages.
Note, as for 5 minutes ago I am squatting the scope features:
Cons:
|
Stef Penner [2:37 PM] I'm concerned about Stef Penner [2:38 PM] More research and exploration needed. |
I've discussed this in the past with numerous core team members and am still unconvinced it should be cut into npm-style packages as the default way to consume. i.e. Sure, if you want to package them separately as that via npm, go for it, but the default ember build that ships with ember-cli should IMO allow you to access everything under the Having experimented with broccoli-babel tree shaking myself, I'm not sure how splitting them into separate npm packages helps automated tree shaking in this way. It's arguably more complex to shake if everything was just on Ultimately, I don't think the developer experience should suffer simply because it's a bit easier to shake the tree at the npm module boundry vs. inner-module boundaries. Side note bikeshed that won't make it, but have to say it: I disagree with ember-cli's convention of hyphenating the filename and thus the fully qualified path of modules. It's using ES2015 modules unlike most module systems and makes things less clear for no apparent benefit. This tradition seems to be carried over to ember core as well. I think there's much benefit to Edit: @rwjblue and I have been discussing this on Slack. Nothing really to add, but I understand the original argument better now though. |
|
I know +1's suck, but I forgot to mention all of what @MajorBreakfast just said I agree with as well. |
I like @stefanpenner's suggestion of the npm namespace, but in the event that doesn't work out I don't see why we can't have the best of both worlds. If Ember itself is going to be distributed as an ember-cli addon, we can override the targeted es6 module prefix to transpile to. So for any number of node packages:
we can set the
Then the |
@bcardarella one downside, is |
@zeppelin Can we also add an export for |
@mixonic Absolutely, will do once I got home! On Mon, Jul 6, 2015 at 6:10 PM Matthew Beale notifications@github.com
|
I believe the general tone is now, We clearly need to fixup some parts of ember-cli that don't quite work with npm scopes. I would love to hear other concerns with this approach, I am sure we are missing something. |
I am not very aware of downsides of each approach, but |
I'm in favor of whichever syntax would not have to require us to change it again in the future. |
@Gaurav0 that would be the point of attempting to enumerate potential issues while in RFC form. Do you see any potential issues we may be missing? |
@stefanpenner I don't feel I understand npm well enough to know why a dash is better than a slash, much less know about any missing potential issues. Just wanted to weigh in that I would prefer not having to change again over something that feels more "natural". |
I'm in favor of @stefanpenner's |
I like the scope proposal. I would like |
@tomdale @stefanpenner how does the import work, |
I'm also in favor of npm scopes and like |
TL;DR
cc @chadhietala i tried to link to your recent graph work, but it appears the most recent may not yet be pushed? |
@stefanpenner Yes that is correct. Still fixing up some tests but it is here. The "prune unreachable" piece is here |
@stefanpenner @chadhietala thanks |
After thinking about this more I don't think taking the approach implemented here is correct because it is not a correct representation of the real graph but rather an explosion of complexity without any real advantage to the end user. I propose that Ember actually builds the packages described in app-shims and starts to be less complected. In the packager work I've introduced the concept of a
dep-graph.json
Downstream in the linker we consume this information to determine what is actually in the graph and thus shaking the broccoli trees to only output a tree that is an actual representation of the real graph. Introducing more shims simply makes dealing with real graph resolution much more difficult because now you need exceptions into the tree shaking process that point at a single shadowing file. |
Thanks for this RFC, I am closing in-favor of: #176 as may of the ideas are incorporated, and you also appear on board #176 (comment) Thanks for helping flesh this out! |
Rendered