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

feat(loader): inject import statements that are statically analyzable #2959

Closed

Conversation

johnjenkins
Copy link
Contributor

@johnjenkins johnjenkins commented Jul 12, 2021

Current behaviour

Stencil provides it's neat lazy-loader - include once then forget 🚀 - it's super useful.
The loader comes in 2 flavours. 1) is designed to be loaded via a <script> in a browser and that's that. Perfect for something like a CDN. 2) is designed to be bundled via a ... bundler:

import { defineCustomElements, applyPolyfills } from '@your-components/components/loader';
applyPolyfills().then(() => defineCustomElements());

Aside from copying static assets from node_modules/@your-components/assets to public/assets that's it.

If you use webpack ^ that should all work nicely.

Try using that in parcel, vite, snopack, esbuild, rollup etc :(

The dynamic import expression that stencil's loader loadModule function ships with:

import(`./${bundleId}.entry.js?${somethingElse}`).then(

Is webpack centric - the other bundlers don't know how to find the resources they need to bundle.

This PR / new behaviour

This small PR adds static imports to the stencil lazy loader's outputs that are not used directly in the browser (e.g. via CDN).
Because the import statements are static strings:

import(`./your-component.entry.js`).then(

All the other bundlers are happy.

How it works

The PR simply adds a comment 'hook' to the client-load-module.ts < where the loadModule resides.
Just before writing the core loader file to disk (which includes the client-load-module.ts module), it replaces the comment with a switch statement using all the potential 'entry modules' (component bundles that are lazy loaded) that are created by stencil to return static import statements.

The static imports do not get added if the core loader is going to be used directly in the browser (e.g. via CDN).
The static imports will / should not apply when stencil is using HMR (the dynamic addition of the HMR ID will break static imports).

Testing

I have manually tested the PRs output against my live component library in:

  • Parcel
  • Vite
  • Snowpack
  • Rollup
  • Webpack (to check that nothing breaks)

If there's any more stuff that the current stencil loader is not working with - please leave a comment here and I'll check it out.

Fixes #2827
Fixes #2752

@johnjenkins johnjenkins changed the title feat(loader): inject static imports that are statically analyzable feat(loader): inject import statements that are statically analyzable Jul 12, 2021
@johnjenkins johnjenkins marked this pull request as ready for review July 14, 2021 23:27
@johnjenkins
Copy link
Contributor Author

johnjenkins commented Jul 15, 2021

To any requiring this sooner, rather than later I built out a temporary package of all my recent PRs - including this ^.

What else:
new @prop getters / setters (#2899)
new @mixindecorator (#2921)
new sourceMap option for all output (including integration with @Mixins) (#2892)
improved non-shadow slot behaviour (#2938)

You can test it out with little disruption by just swapping out any reference to @stencil/core in your package.json
e.g.

"@stencil/core": "2.6.0"

in package.json, becomes

"@stencil/core": "npm:@johnjenkins/stencil-community^2.5.0"

I will keep this package in-sync with @stencil/core master up until the relevant PRs get accepted or rejected.
Many thanks!

@zombieleet
Copy link

I am still having this issues after using your changes

@johnjenkins
Copy link
Contributor Author

thanks so much for trying it out!
Please can I ask for a minimum reproduction repo so I can fix this issue :)

@armenr
Copy link

armenr commented Aug 6, 2021

@johnjenkins -

Update: pnpm install @stencil/core@npm:@johnjenkins/stencil-community

^^ This is a good way to get the package :) but it causes the following


error when starting dev server:
Error: Build failed with 3 errors:
node_modules/.pnpm/@aws-amplify+ui-components@1.7.1_aws-amplify@4.2.2/node_modules/@aws-amplify/ui-components/dist/components/index.js:1:9: error: No matching export in "node_modules/.pnpm/@johnjenkins+stencil-community@2.5.5/node_modules/@johnjenkins/stencil-community/internal/stencil-core/index.js" for import "attachShadow"
node_modules/.pnpm/@aws-amplify+ui-components@1.7.1_aws-amplify@4.2.2/node_modules/@aws-amplify/ui-components/dist/components/index.js:1:32: error: No matching export in "node_modules/.pnpm/@johnjenkins+stencil-community@2.5.5/node_modules/@johnjenkins/stencil-community/internal/stencil-core/index.js" for import "createEvent"
node_modules/.pnpm/@aws-amplify+ui-components@1.7.1_aws-amplify@4.2.2/node_modules/@aws-amplify/ui-components/dist/components/index.js:1:45: error: No matching export in "node_modules/.pnpm/@johnjenkins+stencil-community@2.5.5/node_modules/@johnjenkins/stencil-community/internal/stencil-core/index.js" for import "proxyCustomElement"
    at failureErrorWithLog (/Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:1449:15)
    at /Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:1131:28
    at runOnEndCallbacks (/Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:921:63)
    at buildResponseToResult (/Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:1129:7)
    at /Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:1236:14
    at /Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:609:9
    at handleIncomingPacket (/Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:706:9)
    at Socket.readFromStdout (/Users/armenr/Development/beep-beep-workspace/beep-vitesse-test-2/packages/client/node_modules/.pnpm/esbuild@0.12.17/node_modules/esbuild/lib/main.js:576:7)
    at Socket.emit (node:events:394:28)
    at Socket.emit (node:domain:470:12)
 ERROR  Command failed with exit code 1.

Installing without the npm alias produces:

 ERROR  GET https://registry.npmjs.org/@johnjenkins%2Fstencil-community%5E2.5.5: Not Found - 404
@johnjenkins/stencil-community^2.5.5 is not in the npm registry, or you have no permission to fetch it. Did you mean @johnjenkins/stencil-community^?

@johnjenkins
Copy link
Contributor Author

johnjenkins commented Aug 6, 2021

@armenr sorry to hear that!
weird - It's definitely there! https://www.npmjs.com/package/@johnjenkins/stencil-community
Perhaps install independently and use npm-link instead?

rwaskiewicz and others added 2 commits August 6, 2021 10:16
- upgrade prettier to v2.3.2
  - lock version to prevent breaking changes in minor versions
- add prettier.dry-run package.json script
- add pipeline action to evaluate format status
- add prettierignore file for faster runs

STENCIL-8: Add Prettier to Stencil
@johnjenkins johnjenkins requested a review from a team August 6, 2021 09:17
@deadghost
Copy link

I ran this and it fixed my issue. I needed an extra @ to download the dependency.
"@stencil/core": "npm:@johnjenkins/stencil-community@^2.5.0" instead of
"@stencil/core": "npm:@johnjenkins/stencil-community^2.5.0"

johnjenkins and others added 8 commits September 7, 2021 00:29
…ing-dyn-imports

# Conflicts:
#	.prettierignore
#	package-lock.json
#	package.json
#	scripts/license.ts
#	scripts/release-tasks.ts
#	scripts/release.ts
#	scripts/utils/release-utils.ts
#	src/sys/node/node-sys.ts
#	test/browser-compile/src/utils/css-template-plugin.ts
#	test/karma/stencil.config.ts
#	test/karma/test-app/slot-fallback/karma.spec.ts
#	test/karma/test-app/util.ts
@darrylnoakes
Copy link

+1
I'm trying to use Ionic v6 beta now to get around it.

@adamklein
Copy link

+1, this was exactly the problem I was running into via https://github.com/jepiqueau/jeep-sqlite

@Fanna1119
Copy link

Any progress on the merge for this?

@johnjenkins
Copy link
Contributor Author

johnjenkins commented Feb 3, 2022

sorry - this is on me - I'm super busy in real life atm.

If anyone wants to make a demo repo just showing this PR working with a bunch of different bundlers that can get Ryan what he needs and get this unstuck..

@PrinceManfred
Copy link

I've spun up a repo with webpack, vite, and parcel. The vite production build script uses rollup so that is also kind of demoed. Does this help move things along?

https://github.com/PrinceManfred/stencil-bundler-tests

@jeffplloyd
Copy link

I've recently switched over to using "@stencil/core": "npm:@johnjenkins/stencil-community@^2.12.2" and have tested in a production application using create-vite with the react-ts template and my Stencil components are working as expected with the application running in development. I'm subscribing to this thread in hopes that this will be added to @stencil/core soon! I give this a thumbs up!

@jeco123
Copy link

jeco123 commented Mar 10, 2022

We need it :-)

@rwaskiewicz
Copy link
Member

@PrinceManfred

I've spun up a repo with webpack, vite, and parcel. The vite production build script uses rollup so that is also kind of demoed. Does this help move things along?

thanks! This definitely looks promising. I'll get some time scheduled next sprint to start to circle back to this issue and dive in deeper

@lkononenko
Copy link

Thanks for the PR with Vite support and @rwaskiewicz for picking it up! Do you have any estimations/updates when the Vite support can be released?

@pndiogo
Copy link

pndiogo commented Mar 24, 2022

Thanks so much for tackling this issue. Any updates when it might be merged? It's highly needed.

mfranzke added a commit to mfranzke/stencil that referenced this pull request Mar 25, 2022
@mfranzke
Copy link

mfranzke commented Mar 25, 2022

@johnjenkins could you please update your branch and release a new npm package as well ? There have been stencil releases in the meantime.

@rwaskiewicz
Copy link
Member

Hey folks - quick update - we're going to start looking at this during our next sprint (starting this upcoming Monday).

@jeffplloyd
Copy link

Any movement on this?

@edolix
Copy link

edolix commented May 2, 2022

It would be cool to have this merged in - looking forward for it!

@lkononenko
Copy link

we're also looking forward for it a lot! Thanks for all your work and please let us know in case of any updates ☺️

@AlexanderYW
Copy link

Any news on this one? :)

rwaskiewicz added a commit that referenced this pull request May 9, 2022
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
rwaskiewicz added a commit that referenced this pull request May 9, 2022
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
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 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
rwaskiewicz added a commit that referenced this pull request May 13, 2022
)

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.

add a stencil component library to be used in tests that verify
applications that consume the library and are bundled with vite, parcel,
etc.

add an application that is built using vite to the bundler
test directory. it consumes a small stencil library build using the
`dist` output target, and verifies that the application can load the web
component when the application has been built using vite.

add infrastructure for running the bundler tests in karma.
karma was chosen to align with existing parts of our technical stack
(see the `test/karma` directory), and to expedite the initial
implementation phase of these tests. karma can be difficult to
configure, and even more difficult to add new (i.e. different) testing
paradigms and testing strategies to. given that these tests do not use
browserstack and are a significant departure from the existing karma
tests, it felt 'ok' to split these off into a separate set of tests
(with their own configuration).

in order to get tests up and running, a utilities file,
`test/bundler/karma-stencil-utils.ts` has been created. this file is
largely based off of `test/karma/test-app/util.ts`. parts of the
existing utility file were not ported over if they were deemed
unnecessary, and attempts were made to clean up the existing code to
improve their readability.

wire the bundler tests to github actions. these tests are
kept in a new reusable workflow that can run in parallel with existing
analysis, unit and e2e tests

STENCIL-339: Integrate Bundler Functionality
@rwaskiewicz
Copy link
Member

👋 The core functionality in this PR has been made a part of the v2.16.0 release of Stencil. This functionality is gated under a new flag, experimentalimportinjection. Folks wishing to make use of the functionality should enable this flag and recompile their Stencil projects.

I'm going to close this PR. Once again I wanted to thank @johnjenkins & @PrinceManfred for helping move this along. We couldn't have done it without y'all ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet