fix(compiler): correctly generate CSS rules using ::slotted outside shadow DOM
#4969
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
|
| 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/build/build-stats.ts | 27 |
| src/compiler/output-targets/dist-lazy/generate-lazy-module.ts | 25 |
| src/compiler/style/test/optimize-css.spec.ts | 23 |
| src/testing/puppeteer/puppeteer-element.ts | 23 |
| src/compiler/prerender/prerender-main.ts | 22 |
| src/runtime/client-hydrate.ts | 19 |
| 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/build/build-finish.ts | 13 |
| src/compiler/prerender/prerender-queue.ts | 13 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2345 | 418 |
| TS2322 | 395 |
| TS18048 | 310 |
| TS18047 | 101 |
| TS2722 | 38 |
| TS2532 | 34 |
| TS2531 | 23 |
| TS2454 | 14 |
| TS2352 | 13 |
| TS2769 | 10 |
| TS2790 | 10 |
| TS2538 | 8 |
| TS2344 | 5 |
| TS2416 | 4 |
| TS2493 | 3 |
| TS18046 | 2 |
| TS2684 | 1 |
| TS2488 | 1 |
| TS2430 | 1 |
Unused exports report
There are 15 unused exports on this PR. Unfortunately, it looks like that's an increase of 1 over main 😞.
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 | 232 | resolve |
| src/utils/index.ts | 246 | 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 | 62 | satisfies |
| src/compiler/types/validate-primary-package-output-target.ts | 62 | Record |
| src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
| src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
tanner-reits
commented
Oct 23, 2023
src/utils/regular-expression.ts
Outdated
| * @param text The string potentially containing special characters. | ||
| * @returns The string with all special characters escaped. | ||
| */ | ||
| export const escapeRegExpSpecialCharacters = (text: string) => { |
There was a problem hiding this comment.
Made this a util since I could see this coming up again. If something like this already exists, or we do something similar in other places, I can update accordingly
There was a problem hiding this comment.
I don't know of anything but this looks like it will be handy in the future!
| @@ -471,15 +473,32 @@ const scopeCssText = ( | |||
| cssText = scopeSelectors(cssText, scopeId, hostScopeId, slotScopeId, commentOriginalSelector); | |||
| } | |||
|
|
|||
| cssText = cssText.replace(/-shadowcsshost-no-combinator/g, `.${hostScopeId}`); | |||
| cssText = replaceShadowCssHost(cssText, hostScopeId); | |||
There was a problem hiding this comment.
Tried to reorder this code so we didn't have to worry about doing this replacement in multiple places, but that caused other problems, so figured this was the solution that would cause the least headaches for now
rwaskiewicz
suggested changes
Oct 27, 2023
alicewriteswrongs
approved these changes
Oct 31, 2023
rwaskiewicz
approved these changes
Nov 1, 2023
tanner-reits
deleted the
treits/fix/scope-css-replace-slotted-selectors
branch
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.
What is the current behavior?
Using a
::slottedpseudo-selector outside a shadow DOM component can result in styles not working as expected. The issue is with how we transform the CSS to account for our pseudo-slot behavior (relocation).GitHub Issue Number: #3977
What is the new behavior?
The CSS in transformed into the correct rule(s) to account for nested elements. Also updated the logic so we replace all instances of the pre-transformed string in the CSS text, rather than just the first.
Does this introduce a breaking change?
Testing
Automated tests
Updated the existing tests to account for the change in logic (which is what they should have been in the first place)
Manual testing
Install a build of this branch into the linked issue reproduction case and verify the button styles for the "scoped" button are applied as expected to match the other buttons.
Other information