fix(runtime): add lazy load for addtl bundlers #3364
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this commit adds experimental support for using stencil component libraries in projects that use bundlers such as vite. prior to this commit, stencil component libraries that were used in projects that used such bundlers would have issues lazily-loading components at runtime. this is due to restrictions the bundlers themselves place on the filepaths that can be used in dynamic import statements. this commit does not introduce the ability for stencil's compiler to use bundlers other than rollup under the hood. it only permits a compiled component library (that uses the `dist` output target) to be used in an application that uses a bundler built atop of rollup. due to the restrictions that rollup may impose on dynamic imports, this commit adds the ability to add an explicit `import()` statement for each lazily-loadable bundle. in order to keep the runtime small, this feature is hidden behind a new feature flag, `experimentalImportInjection` this pr build's atop the work done by @johnjenkins in #2959 and the test cases provided by @PrinceManfred in #2959 (comment). Without their contributions, this commit would not have been possible. STENCIL-339: Integrate Bundler Functionality
| isBrowserBuild | ||
| ); | ||
| }) | ||
| ), |
There was a problem hiding this comment.
it looks like moving down the Promise.all here leaves us with two nested Promise.all calls (lines 33 and 34), I think we could consolidate that down to just one while we're in here
There was a problem hiding this comment.
Good catch! Fixed in cd3f228
alicewriteswrongs
approved these changes
May 10, 2022
remove two nested `Promise.all` calls - after moving writing lazily loadable chunks out of the outermost `Promise.all` call, we were awaiting a single promise wrapped in an array. this commit simplifies the code
rwaskiewicz
added a commit
that referenced
this pull request
May 10, 2022
* fix(runtime): add lazy load for addtl bundlers this commit adds experimental support for using stencil component libraries in projects that use bundlers such as vite. prior to this commit, stencil component libraries that were used in projects that used such bundlers would have issues lazily-loading components at runtime. this is due to restrictions the bundlers themselves place on the filepaths that can be used in dynamic import statements. this commit does not introduce the ability for stencil's compiler to use bundlers other than rollup under the hood. it only permits a compiled component library (that uses the `dist` output target) to be used in an application that uses a bundler built atop of rollup. due to the restrictions that rollup may impose on dynamic imports, this commit adds the ability to add an explicit `import()` statement for each lazily-loadable bundle. in order to keep the runtime small, this feature is hidden behind a new feature flag, `experimentalImportInjection` this pr build's atop the work done by @johnjenkins in #2959 and the test cases provided by @PrinceManfred in #2959 (comment). Without their contributions, this commit would not have been possible. STENCIL-339: Integrate Bundler Functionality * review(ap): remove unnecessary nested `Promise.all` calls remove two nested `Promise.all` calls - after moving writing lazily loadable chunks out of the outermost `Promise.all` call, we were awaiting a single promise wrapped in an array. this commit simplifies the code
rwaskiewicz
added a commit
that referenced
this pull request
May 10, 2022
* fix(runtime): add lazy load for addtl bundlers this commit adds experimental support for using stencil component libraries in projects that use bundlers such as vite. prior to this commit, stencil component libraries that were used in projects that used such bundlers would have issues lazily-loading components at runtime. this is due to restrictions the bundlers themselves place on the filepaths that can be used in dynamic import statements. this commit does not introduce the ability for stencil's compiler to use bundlers other than rollup under the hood. it only permits a compiled component library (that uses the `dist` output target) to be used in an application that uses a bundler built atop of rollup. due to the restrictions that rollup may impose on dynamic imports, this commit adds the ability to add an explicit `import()` statement for each lazily-loadable bundle. in order to keep the runtime small, this feature is hidden behind a new feature flag, `experimentalImportInjection` this pr build's atop the work done by @johnjenkins in #2959 and the test cases provided by @PrinceManfred in #2959 (comment). Without their contributions, this commit would not have been possible. STENCIL-339: Integrate Bundler Functionality * review(ap): remove unnecessary nested `Promise.all` calls remove two nested `Promise.all` calls - after moving writing lazily loadable chunks out of the outermost `Promise.all` call, we were awaiting a single promise wrapped in an array. this commit simplifies the code
rwaskiewicz
added a commit
that referenced
this pull request
May 13, 2022
* fix(runtime): add lazy load for addtl bundlers this commit adds experimental support for using stencil component libraries in projects that use bundlers such as vite. prior to this commit, stencil component libraries that were used in projects that used such bundlers would have issues lazily-loading components at runtime. this is due to restrictions the bundlers themselves place on the filepaths that can be used in dynamic import statements. this commit does not introduce the ability for stencil's compiler to use bundlers other than rollup under the hood. it only permits a compiled component library (that uses the `dist` output target) to be used in an application that uses a bundler built atop of rollup. due to the restrictions that rollup may impose on dynamic imports, this commit adds the ability to add an explicit `import()` statement for each lazily-loadable bundle. in order to keep the runtime small, this feature is hidden behind a new feature flag, `experimentalImportInjection` this pr build's atop the work done by @johnjenkins in #2959 and the test cases provided by @PrinceManfred in #2959 (comment). Without their contributions, this commit would not have been possible. STENCIL-339: Integrate Bundler Functionality * review(ap): remove unnecessary nested `Promise.all` calls remove two nested `Promise.all` calls - after moving writing lazily loadable chunks out of the outermost `Promise.all` call, we were awaiting a single promise wrapped in an array. this commit simplifies the code
|
Thanks for an amazing work! Do you have any estimation when it can be released? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
This is going to be the final part part of a chain of PRs*. Once all of them are up/approved, I'll merge the child-most PR into its parent, that parent into its own parent, etc. At the end, we shall have a PR (#3349) which contains all of its children, and we'll commit it as one large commit.
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build) was run locally and any changes were pushednpm test) were run locally and passednpm run test.karma.prod) were run locally and passednpm run prettier) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Supersedes: #2959
What is the new behavior?
this commit adds experimental support for using stencil component
libraries in projects that use bundlers such as vite. prior to this
commit, stencil component libraries that were used in projects that used
such bundlers would have issues lazily-loading components at runtime.
this is due to restrictions the bundlers themselves place on the
filepaths that can be used in dynamic import statements.
this commit does not introduce the ability for stencil's compiler to use
bundlers other than rollup under the hood. it only permits a compiled
component library (that uses the
distoutput target) to be used in anapplication that uses a bundler built atop of rollup.
due to the restrictions that rollup may impose on dynamic imports, this
commit adds the ability to add an explicit
import()statement for eachlazily-loadable bundle. in order to keep the runtime small, this feature
is hidden behind a new feature flag,
experimentalImportInjectionthis pr build's atop the work done by @johnjenkins in
#2959 and the test cases
provided by @PrinceManfred in
#2959 (comment).
Without their contributions, this commit would not have been possible.
STENCIL-339: Integrate Bundler Functionality
Does this introduce a breaking change?
Testing
Other Information
Since this is a part of a chain, please do not use GH to 'Update Branch' - I'll take care of that manually (and be careful not to rebase mid-review cycle).
Upon merging the entire chain, I'll update GH to require the bundler tests to merge (so they won't show up in the list of passed checks in the PR summary down below)
Footnotes
* I am going to be assuming tech debt for getting Parcel-based tests into this chain, and am going to defer to those at a later time