-
Notifications
You must be signed in to change notification settings - Fork 785
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(compiler): fix generated import statement #5419
Conversation
STENCIL-855
|
Path | Location | Error | Message |
---|---|---|---|
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts | (24, 12) | TS18048 | |
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts | (25, 12) | TS18048 | |
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts | (43, 12) | TS18048 | |
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts | (44, 12) | TS18048 | |
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts | (47, 12) | TS18048 |
reports and statistics
Our most error-prone files
Path | Error Count |
---|---|
src/dev-server/index.ts | 37 |
src/dev-server/server-process.ts | 32 |
src/compiler/prerender/prerender-main.ts | 22 |
src/testing/puppeteer/puppeteer-element.ts | 22 |
src/runtime/client-hydrate.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/compiler/transpile/transpile-module.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/mock-doc/test/attribute.spec.ts | 12 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2322 | 361 |
TS2345 | 349 |
TS18048 | 206 |
TS18047 | 82 |
TS2722 | 37 |
TS2532 | 24 |
TS2531 | 21 |
TS2454 | 14 |
TS2790 | 11 |
TS2352 | 10 |
TS2769 | 8 |
TS2538 | 8 |
TS2416 | 6 |
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/8164820967/artifacts/1300436663 If your browser saves files to
|
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.
👍 for more tests! I had some asks around type safety, take a look at the included patch and LMK what you think
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts
Show resolved
Hide resolved
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts
Outdated
Show resolved
Hide resolved
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts
Outdated
Show resolved
Hide resolved
src/compiler/output-targets/dist-custom-elements/test/dist-custom-elements.spec.ts
Outdated
Show resolved
Hide resolved
@rwaskiewicz thanks for the patch, good to know these primitives exist, that helps a lot. I added another test for the "non-plain" use case. |
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.
LGTM
What is the current behavior?
If you have a plain component like this:
and try to export it via:
the compiler will fail with:
The change was originally introduced here: https://github.com/ionic-team/stencil/pull/3368/files#diff-df2741ac09fef4bbb750d864b2b96bdc4f8e649f347c7cc7def8edc977e1004eR198 and seems to be not intentional.
What is the new behavior?
Removed the additional bracket.
Documentation
n/a
Does this introduce a breaking change?
Testing
I added a unit test for this particular use case.
Other information
n/a