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

Improve External Module Bundling and Load Through ES Module Imports #162

Closed
adamdbradley opened this issue Sep 16, 2017 · 19 comments
Closed
Assignees
Milestone

Comments

@adamdbradley
Copy link
Contributor

Build a graph of all the shared modules and rollup each bundle as far down as possible. Then use ES module imports to load the shared bundles. For browsers that do no support ES modules it'll fallback to the current process.

@kristianmandrup
Copy link

So what to do until this issue is resolved? Any recommendations?

@kylecordes
Copy link

kylecordes commented Dec 3, 2017

I think this is already implied by the description, but I'll write it out just in case....

Hopefully the ultimate goal here is effective sharing of library code across multiple separately developed Stencil components and further across Stencil consuming code:

  • Developer A publishes a Stencil component that uses RxJS
  • Developer B publishes a Stencil component that uses RxJS
  • Developer C uses Angular, which uses RxJS
  • Developer C's application directly uses RxJS, and is written with Angular, and uses the Stencil components from A and B
  • When it all ends up running on the page, there is only a single copy of RxJS shared by all of the above

This should be implementable today for current browsers using ES modules; fallback to static bundling would need to happen at the highest (application) level to get correct results for all browsers. I don't see a way to get correct results for all browsers if the Stencil build does the fallback bundling. You would end up with (my in my example) more than one copy of RxJS on the page.

@felschr
Copy link

felschr commented Dec 7, 2017

Any update on this?

@adamdbradley
Copy link
Contributor Author

@felschr Yup, we've got a branch going and actively workin on it.

@adamdbradley adamdbradley modified the milestones: 0.0.8 - ES Module Imports, 0.0.9 - Dynamic imports() Dec 8, 2017
@jgw96
Copy link
Contributor

jgw96 commented Dec 21, 2017

Just wanted to post an update here as this issue is getting a decent amount of traction on the stencil slack. The team is actively working on this and we are pretty close to having it working. This is probably our biggest refactoring of the internals of the Stencil compiler to date, or atleast one of the biggest, so its taking some time.

@jgw96 jgw96 mentioned this issue Dec 27, 2017
1 task
@rolandjitsu
Copy link

Is this solved now?

@kristianmandrup
Copy link

@JGW This is likely the most anticipated update as well. Not using Stencil until this critical issue is resolved. What is the timeline? Before end of January? Alpha, Beta?
Thanks :)

@rolandjitsu
Copy link

rolandjitsu commented Jan 17, 2018

@kristianmandrup I might be mistaken, but it seems like the built files do not contain the imports that I have in my components. That leads me to believe that this is partially solved?

At least the part where it used to bundle all the imported libs together with the components seems to be solved ... not sure if importing the components works now.

@kristianmandrup
Copy link

What is the expected timeline on this? You expect to have a beta release of this critical feature/improvement end of January 2018?

Thanks :)

@adamdbradley adamdbradley modified the milestones: 0.2.0, 0.3.0 Jan 18, 2018
@adamdbradley
Copy link
Contributor Author

@jthoms1 is actively working on a branch. The actual ES module part of it is working, the remaining bits is the alternate build for ie11. I just moved this to the 0.3.0 so we can ship other features in the 0.2.0 and not let this hold anything else up. Thanks

@kristianmandrup
Copy link

Great! Please don't let IE support hold it back ;) IE is a minor browser these days

@adamdbradley
Copy link
Contributor Author

That said, maybe we can ship some test versions full well knowing legacy modules won't work. @jthoms1 thoughts?

@jthoms1
Copy link
Contributor

jthoms1 commented Jan 18, 2018

The esmodule code splitting is within rollup PR rollup/rollup#1841. I have been working with it locally and it works really well. I am not comfortable shipping this until rollup actually does a release with the feature. There is a lot activity so I would assume it should be soon.

@kristianmandrup
Copy link

kristianmandrup commented Jan 24, 2018

lukastaegert merged commit 6213e9a into release-0.55.0 3 days ago

So it looks like it was recently merged and available from release 0.55 which was released yesterday (Jan 23rd). Good to go!!! :)

I'm super excited to finally use this feature/improvement on my next project. Cheers! :)

@jthoms1
Copy link
Contributor

jthoms1 commented Jan 25, 2018

Now waiting on this PR top get merged. rollup/rollup-plugin-commonjs#283

@jthoms1 jthoms1 mentioned this issue Jan 29, 2018
@jthoms1 jthoms1 closed this as completed Jan 29, 2018
@FlawaCLV
Copy link

@jthoms1 could you provide with an example maybe I'm missing something here... Because when I try to import import { Observable } from 'rxjs/Rx'; the compiler throw [ WARN ] 'default' is not exported by 'node_modules/rxjs/add/operator/toPromise.js'

Any idea ?

@jthoms1
Copy link
Contributor

jthoms1 commented Feb 16, 2018

Looks like this is a known issue. rollup/rollup-plugin-commonjs#255

According to the comments this might warn but should still work. Is this what you are experiencing?

@kael
Copy link

kael commented Feb 17, 2018

I confirm that [ WARN ] 'default' is not exported by 'node_modules/rxjs/add/operator/toPromise.js' is a non-blocking warning, Observable can be imported and then used.

@FDiskas
Copy link

FDiskas commented May 7, 2020

rollup/rollup-plugin-commonjs#283 is now merged, what about this proposal?

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

No branches or pull requests

10 participants