feat(renderer): add flag to update default slot text content #5354
Conversation
add "experimentalDefaultSlotTextContentFix" flag to update text content of the default slot instead of update all the content fixes one of the issues raised in ionic-team#5335
yigityuce
force-pushed
the
feat/default-slot-text-content
branch
from
bc7023e
to
2dac0bd
Compare
yigityuce
changed the title
|
| 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 | 350 |
| TS18048 | 201 |
| TS18047 | 91 |
| TS2722 | 37 |
| TS2532 | 24 |
| TS2531 | 22 |
| 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 |
rwaskiewicz
added
the
Resolution: Refine
PR built and packed!Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8032920823/artifacts/1272284462 If your browser saves files to |
adds check to handle the edge case where textContent is set before component is loaded
|
Thanks for the contribution @yigityuce 🙏 We have discussed this idea/proposal with the team and have decided that this is not something we would like to support. It creates a behavior that contradicts with how the Shadow DOM behaves which is not a direction we would like to take. Furthermore we believe that you can achieve your goals by modifying the query in your example, e.g.: <body>
<my-component>
<span slot="prefix">Prefix</span>
<span slot="suffix">Suffix</span>
<span>Content text</span>
</my-component>
<button onclick="onButtonClick()" style="margin-top: 48px">Update content</button>
</body>
<script>
function onButtonClick() {
const defaultSlot = document.querySelector('my-component span:not([slot])');
defaultSlot && (defaultSlot.textContent = `${Date.now()}`);
}
</script>Is there any reason this wouldn't work for you? |
What if I don't wrap the "Content text" with |
|
EDIT: this case is not valid, please IGNORE this message and one more example that comes up my mind is like that: // component lib file
@Component({ tag: 'my-component' })
export class MyComponent {
@Event() randomize: EventEmitter<void>;
@Event() reset: EventEmitter<void>;
render() {
return (
<Host>
<button onClick={() => this.randomize.emit()}>Randomize</button>
<slot />
<button onClick={() => this.reset.emit()}>Reset</button>
</Host>
);
}
}// in react context
export const Example: FC = () => {
const [counter, setCounter] = useState(0);
return (
<>
<my-component onReset={() => setCounter(0)} onRandomize={() => setCounter(Date.now() % 10)}>
{counter}
</my-component>
<button onClick={() => setCounter(counter + 1)}>Increment</button>
</>
);
}<!-- Expected rendered DOM after clicking increment button -->
<my-component>
<button>Randomize</button>
1
<button>Reset</button>
</my-component>
<button>Increment</button>
<!-- Actual rendered DOM after clicking increment button -->
<my-component>
1
</my-component>
<button>Increment</button>because current behavior is to remove all nodes first and then insert an new text to to the host |
Can you share this as a Stackblitz project? |
I tried to reproduce but I realized it is my bad, it is NOT a valid case, so pls ignore that example. However, during this try-out, I found out another edge case with the given query selector to query the not slotted element. I directly copied the content of the files here to explain and also created a new branch with an updated version of the repro repo. @Component({
tag: 'my-component',
styleUrl: 'my-component.css',
shadow: false,
scoped: true,
})
export class MyComponent {
render() {
return (
<Host>
<div class="some-fancy-styles" id="prefix-wrapper">
<slot name="prefix" />
</div>
<slot />
<div class="some-fancy-styles" id="suffix-wrapper">
<slot name="suffix" />
</div>
</Host>
);
}
}<!DOCTYPE html>
<html dir="ltr" lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0" />
<title>Stencil Component Starter</title>
<script type="module" src="/build/stencil-v4-sc-default-slot-textcontent-update.esm.js"></script>
<script nomodule src="/build/stencil-v4-sc-default-slot-textcontent-update.js"></script>
</head>
<body>
<my-component>
<span slot="prefix">Prefix</span>
<span slot="suffix">Suffix</span>
<div>Content text</div>
</my-component>
<button onclick="onButtonClick()" style="margin-top: 48px">Update content</button>
</body>
<script>
function onButtonClick() {
const contentTextWrapper = document.querySelector('my-component div:not([slot])');
console.log('not slotted:', contentTextWrapper)
contentTextWrapper && (contentTextWrapper.textContent = `${Date.now()}`);
}
</script>
</html>so the thing is when you hit the button, the The only way that I see to accomplish this is to provide a unique selector to select Or, a big or, being able to update only the default slot element while mutating the custom component's textContent (which is the solution I propose). What would be the drawbacks of this proposal and the mitigation steps:
Summary: I mean, removing all the content & updating the custom component's textContent has a workaround, but it is not the case for the default slot with textNode. One more thing might work in future but it's not usable as of today because of the browser compatibility: ::target-text |
Try to make the selector more strict, e.g.
The drawbacks that caused the team to push back on this proposal are:
I am not sure if this selector actually addresses this. I feel inclined to close the issue. Please see above reasons why we don't feel comfortable merging the proposal. The DOM API provides various options that allows to query the element you like to update. We don't see any need to provide workarounds that introduce contradicting behavior to the Shadow DOM spec. |
|
I understand your concerns and you have a valid point so I'm closing this PR. Thanks for the discussion and for showing the point of your view clearly. |
add "experimentalDefaultSlotTextContentFix" flag to update text content of the default slot instead of update all the content
fixes one of the issues raised in #5335
What is the current behavior?
As of now, when the "experimentalScopedSlotChanges" flag is set, it removes all the content of the custom component (including named slots) and then inserts the text.
GitHub Issue Number: #5335
What is the new behavior?
When the newly introduced "experimentalDefaultSlotTextContentFix" is set, updating
textContentupdates only the "default slot" content while keeping the named slot contents.Documentation
Does this introduce a breaking change?
Testing
Other information