-
Notifications
You must be signed in to change notification settings - Fork 795
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(slot-fallback): fix hiding fallback slot content issue when the slotted element is a text node #5496
fix(slot-fallback): fix hiding fallback slot content issue when the slotted element is a text node #5496
Conversation
…lotted element is a text node
|
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/8787416896/artifacts/1436300591 If your browser saves files to
|
…textnode-slot-fallback
@yigityuce I tried to apply the changes in the reproduction case and ran into this: I don't think this is intentional, right? |
…textnode-slot-fallback
No, definitely shouldn't be like this. Could you pls share your env with me somehow, since I tried the fix by creating a local build and I don't see your case with the configuration below: export const config: Config = {
namespace: 'stencil-v4-sc-slot-fallback-with-textnode',
extras: {
experimentalSlotFixes: true,
experimentalScopedSlotChanges: true,
},
};
|
@christian-bromann any update? |
Unfortunately not yet. I will have a response for you early next week. |
@yigityuce Hey 👋 so here are my steps to reproduce my finding:
Can you confirm you see the same behavior? |
I tried but unfortunately I cannot reproduce, here are the steps below that I am following. Could you pls try in that way? git clone https://github.com/yigityuce/stencil.git
cd stencil
git checkout fix/textnode-slot-fallback
npm run clean && npm run build && npm pack
cd ..
git clone https://github.com/yigityuce/stencil-v4-sc-slot-fallback-with-textnode.git
cd stencil-v4-sc-slot-fallback-with-textnode
npm uninstall @stencil/core
npm i ../stencil/stencil-core-{xyz}.tgz.zip
npm run start |
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.
@yigityuce sorry, my bad. It appears that NODE_TYPE
is being replaced by the compiler with its actual value and isn't defined copying the changes over. Anyway I was able to now confirm that given changes will fix observed behavior.
Before we give this to the team, can we make some updates to the test?
@christian-bromann all resolved! |
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 👍
I will have someone from the @ionic-team/stencil team take another look at it.
Thanks!
Released in Stencil ♨️ v4.17.0 @yigityuce thanks for your contribution! |
What is the current behavior?
When a text node is provided to the scoped component as a slot, the slot fallback content does not become hidden.
Reproduction repo: https://github.com/yigityuce/stencil-v4-sc-slot-fallback-with-textnode/commits/master/
GitHub Issue Number: #5335
What is the new behavior?
Fixes the issue
Documentation
Does this introduce a breaking change?
Testing
Other information