add defer hydration support to fast-element #6441
Merged
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
…be manually cached and re-applied during connection
📊 Tachometer Benchmark ResultsSummaryclickTrigger10x
create10k
createDelete5x
runFile1k
update10th
usedJSHeapSize
Resultsobservable-runFile1k
runFile1k
usedJSHeapSize
render-create10k
create10k
usedJSHeapSize
render-createDelete5x
createDelete5x
usedJSHeapSize
render-update10th
update10th
usedJSHeapSize
repeat-basic-splice-itemCount=1000&deleteCount=20&addCount=20
clickTrigger10x
usedJSHeapSize
repeat-nested-push-itemCount=100&addCount=20
clickTrigger10x
usedJSHeapSize
repeat-nested-reverse-itemCount=100
clickTrigger10x
usedJSHeapSize
repeat-nested-shift-itemCount=100
clickTrigger10x
usedJSHeapSize
repeat-nested-unshift-itemCount=100&addCount=20
clickTrigger10x
usedJSHeapSize
|
packages/web-components/fast-element/src/components/element-controller.ts
Outdated
Show resolved
Hide resolved
nicholasrice
force-pushed
the
users/nirice/defer-hydration
branch
from
a115c0e to
829d530
Compare
nicholasrice
force-pushed
the
users/nirice/defer-hydration
branch
from
1cbc1a3 to
436fdad
Compare
EisenbergEffect
suggested changes
Oct 19, 2022
| delete (element as any)[propertyName]; | ||
| boundObservables[propertyName] = value; | ||
| delete element[propertyName]; | ||
| element[propertyName] = value; |
There was a problem hiding this comment.
It occurs to me that this will cause all the change handlers to fire during the constructor.
There was a problem hiding this comment.
oops I missed this while reverting, this should be unchanged.
There was a problem hiding this comment.
huh... I feel like a commit got lost somehow. I'll fix
| ElementController, | ||
| ElementControllerStrategy, | ||
| } from "./components/element-controller.js"; | ||
| export { addHydrationSupport } from "./components/hydration.js"; |
There was a problem hiding this comment.
I think the hydration support should not be part of the core but rather provided as an optional through export path.
|
@EisenbergEffect The property re-assignment is reverted now; not sure what happened there, I thought I did that already. I also added two export paths for hydration. The first exports the controller and the second installs it as a side effect. I added both to support conditional installation, which I don't believe is possible with ESM (can't await at top level with |
EisenbergEffect
approved these changes
Oct 19, 2022
| @@ -461,12 +470,24 @@ export class ElementController<TElement extends HTMLElement = HTMLElement> | |||
| if (definition === void 0) { | |||
| throw FAST.error(Message.missingElementDefinition); | |||
| } | |||
| if (elementControllerStrategy === undefined) { | |||
There was a problem hiding this comment.
Instead of doing this should we just set the strategy right after the close of the class definition? Then we don't have to do the undefined check every time. Similar to the way we are handling the style strategy I think.
There was a problem hiding this comment.
sure, yeah that makes sense
| */ | ||
| public static setStrategy(strategy: ElementControllerStrategy) { | ||
| elementControllerStrategy = strategy; | ||
| } | ||
| } |
There was a problem hiding this comment.
Set strategy right here?
nicholasrice
force-pushed
the
users/nirice/defer-hydration
branch
from
35b712e to
ff7b237
Compare
chrisdholt
approved these changes
Oct 20, 2022
janechu
pushed a commit
that referenced
this pull request
Jun 10, 2024
* adding mutation observer and starting to refactor fast-element * break observable re-binding into it's own method * re-apply observable values in constructor so that they don't need to be manually cached and re-applied during connection * revert unnecessary binding observer changes * refactor and reogranize draft implementation, adding tests * added features to exports and updated api report * Change files * align StyleStrategy and ElementControllerStrategy implementations * fix api report * revert observable property re-assignment * re-organize hydration support into export paths * leverage strategy API internally * make mutation observer internal for now * revert removal of context export path * fix unit test failure * perform default strategy assignment as file side effect Co-authored-by: nicholasrice <[email protected]>
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.
Pull Request
📖 Description
This PR illustrates a general approach to supporting defer-hydration in fast-element. The goal was to support hydration scenarios while minimally impacting scenarios that do not need SSR hydration.
To support hydration, client-side JS would set up support prior to defining custom elements:
This PR adds a new configuration concept,
ElementControllerStrategy, which is leveraged to construct each element instancesElementController. The strategy is configured to return aHydrationElementController, an extension ofElementControllerwhich delays theconnect()super call until thederfer-hydrationattribute is removed. Over time, this class will evolve to perform hydration of declarative shadow DOM rather than re-creating template DOM.🎫 Issues
👩💻 Reviewer Notes
📑 Test Plan
✅ Checklist
General
$ yarn changeComponent-specific
⏭ Next Steps