-
Notifications
You must be signed in to change notification settings - Fork 781
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
fix(cli): prevent generate task from crashing #5693
Conversation
|
Path | Error Count |
---|---|
src/dev-server/index.ts | 37 |
src/dev-server/server-process.ts | 32 |
src/compiler/prerender/prerender-main.ts | 22 |
src/runtime/client-hydrate.ts | 20 |
src/testing/puppeteer/puppeteer-element.ts | 20 |
src/screenshot/connector-base.ts | 19 |
src/runtime/vdom/vdom-render.ts | 17 |
src/dev-server/request-handler.ts | 15 |
src/compiler/prerender/prerender-optimize.ts | 14 |
src/compiler/sys/stencil-sys.ts | 14 |
src/sys/node/node-sys.ts | 14 |
src/compiler/prerender/prerender-queue.ts | 13 |
src/compiler/sys/in-memory-fs.ts | 13 |
src/runtime/connected-callback.ts | 13 |
src/runtime/set-value.ts | 13 |
src/compiler/output-targets/output-www.ts | 12 |
src/compiler/transformers/test/parse-vdom.spec.ts | 12 |
src/compiler/transformers/transform-utils.ts | 12 |
src/compiler/transpile/transpile-module.ts | 12 |
src/mock-doc/test/attribute.spec.ts | 12 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2322 | 361 |
TS2345 | 344 |
TS18048 | 204 |
TS18047 | 82 |
TS2722 | 37 |
TS2532 | 24 |
TS2531 | 21 |
TS2454 | 14 |
TS2790 | 11 |
TS2352 | 9 |
TS2769 | 8 |
TS2538 | 8 |
TS2416 | 7 |
TS2493 | 3 |
TS18046 | 2 |
TS2684 | 1 |
TS2430 | 1 |
Unused exports report
There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
File | Line | Identifier |
---|---|---|
src/runtime/bootstrap-lazy.ts | 21 | setNonce |
src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
src/testing/testing-utils.ts | 198 | withSilentWarn |
src/utils/index.ts | 145 | CUSTOM |
src/utils/index.ts | 269 | normalize |
src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
src/compiler/app-core/app-data.ts | 25 | BUILD |
src/compiler/app-core/app-data.ts | 115 | Env |
src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
src/compiler/types/validate-primary-package-output-target.ts | 61 | satisfies |
src/compiler/types/validate-primary-package-output-target.ts | 61 | Record |
src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
PR built and packed!Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8803489431/artifacts/1440079643 If your browser saves files to
|
this pr is motivated by a scneario where running `npx stencil generate` would result in either an error or an early return: ``` npm run generate > gen-test-1@0.0.1 generate > stencil generate [49:23.1] @stencil/core [49:23.2] v4.17.0 ♨️ ``` note: the reported error was never reproduced locally, only the early return. technically, this was introduced in v4.17.0, when we switched over to using esbuild for production builds. however, this has been present in any build that is esbuild-based since we migrated the cli submodule to esbuild (where we had a mixed rollup-esbuild build during the migration for dev builds). removing this alias eliminates dynamic imports in the CJS output of the cli submodule: ``` const { prompt: r } = await import('../sys/node/prompts.js') ``` which wouldn't run properly, causing the error/early return. this change does cause an increase in bundle size, as we end up importing more of prompts.js than we do. to mitigate this, we direct the cli module to the export we actually use (leading to less getting bundled in the cli module). fixes: #5692 STENCIL-1294 cannot identified the prompts.js as a function? ?
2955a45
to
d25b0c1
Compare
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.
👍
What is the current behavior?
See 'new behavior'
GitHub Issue Number: N/A
What is the new behavior?
this pr is motivated by a scenario where running
npx stencil generate
would result in either an error or an early return:
note: the reported error was never reproduced locally, only the early
return.
technically, this was introduced in v4.17.0, when we switched over to
using esbuild for production builds. however, this has been present in
any build that is esbuild-based since we migrated the cli submodule to
esbuild (where we had a mixed rollup-esbuild build during the migration
for dev builds).
removing this alias eliminates dynamic imports in the CJS output of the
cli submodule:
which wouldn't run properly, causing the error/early return.
this change does cause an increase in bundle size, as we end up
importing more of prompts.js than we do. to mitigate this, we direct
the cli module to the export we actually use (leading to less getting
bundled in the cli module).
fixes: #5692
STENCIL-1294 cannot identified the prompts.js as a function? ?
Documentation
N/A
Does this introduce a breaking change?
Testing
npm init stencil && cd $_ && npm i
npm run generate
<- Should fail/return earlynpm i @stencil/core@4.17.0-dev.1713882968.4a08d0e
npm run generate
<- Works!Other information