-
Notifications
You must be signed in to change notification settings - Fork 109
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
Support for tree shaking? #121
Comments
Webpack will only shake out individual exports if the library in question marks itself as having no side effects. See https://github.com/webpack/webpack/blob/master/examples/side-effects/README.md Beyond that, nothing special in ember-auto-import should be needed. |
Yeah I added sideEffets in the library and still no luck. Do you have an example of tree shaking working will with ember + ember auto import ? |
also FYI, I frequently use babel-plugin-transform-imports which works well. But in ember it is not taking the config. I installed the let app = new EmberApp(defaults, {
babel: {
plugins: [
["transform-imports", {
"library-name": {
"transform": "unexisting/path/${member}"
}
}]
]
}
}); Ember builds successfully with this config. I was expecting it to fail since the transformation leads to an non-existing path. To prevent big bundles, I'm trying to transform:
to
|
I have the same problem with date-fns 2.x. From docs (https://date-fns.org/v2.10.0/docs/ECMAScript-Modules):
When I do import like that sideEffects is false: https://unpkg.com/[email protected]/isAfter/package.json, https://unpkg.com/[email protected]/esm/isAfter/package.json |
👋, I also saw the same problem with I've found that it has something with Babel via // ember-cli-build.js
'use strict';
const EmberApp = require('ember-cli/lib/broccoli/ember-app');
module.exports = function(defaults) {
let app = new EmberApp(defaults, {
'ember-cli-babel': {
compileModules: false
}
});
return app.toTree();
}; It looks like webpack sees already compiled output without ES imports and thus it can not do tree shaking. @ef4, what do you think? It is possible to use "full" paths like |
That is interesting! We are supposed to already be turning that off here (for stage1 compilation of addons): and here (for stage3 compilation of everything): The only place where I know that some things still undergo module-to-amd compilation before reaching webpack is in certain addons that have their own custom build pipelines that run babel explicitly. We still allow them to do that so they don't break. Maybe you can debug further and see why these settings don't seem to be taking effect. |
I'm confused, because I have a fresh Ember application created by Ember CLI v3.16.1, and the
I'm not sure how these settings can be applied without that package being installed? EDIT This is for using |
Sorry, I was the one confused, I mixed up my bug reports. I was thinking of embroider, not ember-auto-import. 😛 (The reason you have some embroider packages even in a standard app is that ember-auto-import already shares some embroider code.) This is a good catch. I suspect this was a regression caused when we enabled babel transpilation of all auto-imported modules. It should be straightforward to fix. The cleanest solution is to make sure that when we apply babel to auto-imported dependencies, we're only running |
No worries, and thanks for the explanation 👍. I have to apologise, because after more investigation made today, I have to take back my statement about
I do think, that this is already happening and it is working as it should https://github.com/ef4/ember-auto-import/blob/master/packages/ember-auto-import/ts/webpack.ts#L128-L156. I also do think now is that Given this application code.// app/routes/application.js
import Route from '@ember/routing/route';
import { addDays } from 'date-fns'; Splitter knows about the `date-fns` specifier, but it does not take care of individual imported exports (named or default). {
"app": {
"static": [
{
"specifier": "date-fns",
"entrypoint": "/Users/bobisjan/Developer/ember-date-fns/node_modules/date-fns/esm/index.js",
"importedBy": [
{
"package": "ember-date-fns",
"path": "ember-date-fns/routes/application.js",
"isDynamic": false
}
]
}
],
"dynamic": []
},
"tests": {
"static": [],
"dynamic": []
}
} This generates entrypoint like this, but when webpack sees `require` for `date-fns`, it does not know that only `addDays` from that file should be included, hence it includes everything.if (typeof document !== 'undefined') {
__webpack_public_path__ = (function(){
var scripts = document.querySelectorAll('script');
return scripts[scripts.length - 1].src.replace(/\/[^/]*$/, '/');
})();
}
module.exports = (function(){
var d = _eai_d;
var r = _eai_r;
window.emberAutoImportDynamic = function(specifier) {
return r('_eai_dyn_' + specifier);
};
d('date-fns', [], function() { return require('/Users/bobisjan/Developer/ember-date-fns/node_modules/date-fns/esm/index.js'); });
})(); Does this make sense? |
Yes, definitely. Thanks for investigating. That explains it. We can fix this. The solution has two parts. Part one is extending the splitter so it keeps track of all the names that are consumed by each import statement. That it relatively straightforward. It would add a new property to the Part two is a little more involved. Today, when we generate the webpack entrypoint we emit code like this: d("some-library", [], function() { return require("./node_modules/some-library/dist/index.js") }); The code lives in what ember-auto-import calls its For webpack to see the individual imports, we can't use AMD require, we need to use actual ES imports. But imports can't be synchronous and dynamic, and we don't want to eagerly evaluate every dependency. So instead, I think we need to introduce an extra level of modules. The code above would change to: d("some-library", [], function() { return require("./some-library.js") }); And we would also emit export { only, the, used, names } from "./node_modules/some-library/dist/index.js"; |
Workaround until embroider-build/ember-auto-import#121 is resolved.
Hi @ef4, I see that there are plans for ember-auto-import v2. Do you think this issue will be resolved in v2? If not, is your proposed fix still valid and compatible w/ v2? |
The proposed fix would still work. This hasn't been prioritized, but if anyone wants to work on it that would be good. It's also non-breaking, so there's no pressure to squeeze it into 2.0. |
Hello everyone! thanks for participating in this issue report! 🎉 Just wanted to ask if this is still an issue with ember-auto-import 2.6.3? thanks!! |
@NullVoxPopuli It's still an issue, yes. I've reproduced it in a Stackblitz: https://stackblitz.com/edit/github-bmw1gc |
I'm new to the ember community. I'm trying to make sure my bundle is as small as it can be. I'm using a library that uses named exports so that webpack 4 handles tree shaking well.
I'm importing the library like this:
import { MyComponent } from "library-name"
Inside
library-name
there are a bunch of components being exported, and the final bundle includes everything.I'm trying to figure out how to make sure my final bundle only include
MyComponent
.I'm not entirely sure if this is a problem with
ember-auto-import
, since I'm new to the community.The text was updated successfully, but these errors were encountered: