-
Notifications
You must be signed in to change notification settings - Fork 789
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): ensure not to import duplicate style identifier #5119
Conversation
|
Path | Error Count |
---|---|
src/dev-server/index.ts | 37 |
src/mock-doc/serialize-node.ts | 36 |
src/dev-server/server-process.ts | 32 |
src/compiler/style/test/optimize-css.spec.ts | 23 |
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts | 22 |
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 | 18 |
src/compiler/config/test/validate-paths.spec.ts | 16 |
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/runtime/vdom/vdom-annotations.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/compiler/transformers/transform-utils.ts | 13 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2345 | 399 |
TS2322 | 374 |
TS18048 | 286 |
TS18047 | 101 |
TS2722 | 37 |
TS2532 | 31 |
TS2531 | 23 |
TS2454 | 14 |
TS2352 | 13 |
TS2790 | 10 |
TS2769 | 8 |
TS2538 | 8 |
TS2416 | 6 |
TS2344 | 5 |
TS2493 | 3 |
TS2488 | 2 |
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 |
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!
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.
all looks good to me!
We recently landed changes in main (not released) related to support multiple style urls. These changes however have caused failures when building upstream project, e.g. in Ionic framework (see https://github.com/ionic-team/ionic-framework/actions/runs/7064837482/job/19233697164). This patch reverts these changes to unblock upcoming releases. Revert "fix(compiler): ensure not to import duplicate style identifier (#5119)" This reverts commit 7591dce. Revert "fix(compiler): support multiple styleUrls in components (#5090)" This reverts commit 54e52da.
We recently landed changes in main (not released) related to support multiple style urls. These changes however have caused failures when building upstream project, e.g. in Ionic framework (see https://github.com/ionic-team/ionic-framework/actions/runs/7064837482/job/19233697164). This patch reverts these changes to unblock upcoming releases. Revert "fix(compiler): ensure not to import duplicate style identifier (#5119)" This reverts commit 7591dce. Revert "fix(compiler): support multiple styleUrls in components (#5090)" This reverts commit 54e52da.
What is the current behavior?
We have experienced a failing nightly build due to duplicate style imports caused by a component that is defined as following:
This caused the import using two identical identifier:
GitHub Issue Number: N/A
What is the new behavior?
I tweaked the compiler to include the
styleId
of the component into the identifier which will now resolve into different import identifier:Furthermore I realized a possible edge case where duplicate style urls would cause the same issue, e.g.:
The duplicate styles are now removed.
Lastly due to a lot of code duplication I merged the function that would create the import statements for CJS and ESM into a single one.
Does this introduce a breaking change?
Testing
I found the unit tests to be the best approach to test this behavior. I added some to cover this new edgecase.
Other information
Link to failing Ionic build: https://github.com/ionic-team/ionic-framework/actions/runs/7056427050/job/19208391427