-
Notifications
You must be signed in to change notification settings - Fork 113
refactor: save dataset on an attribute #981
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
Conversation
On lynx, the `data-*` attributes have different behaviors than the HTMLElement has. The dataset will be treated as properties, the key will not be applied the camel-case <-> hyphenate name transformation. Before this commit we use it as a runtime data, but after this commit we will use encodeURI(JSON.stringify(dataset)) to encode it as a string. Implement SSR for Web #52
🦋 Changeset detectedLatest commit: 3b6d8a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Pull Request Overview
This PR refactors how element datasets are stored and accessed by persisting them in a custom l-dset attribute rather than in runtime memory, enabling SSR support.
- Switch
createCrossThreadEventand event functions to readdatasetanduniqueIdfrom element attributes. - Refactor attribute/property APIs to encode/decode dataset via
l-dsetand update nativedata-*attributes. - Remove in-memory
lynxDatasetfrom runtime info and initialize dataset entirely through attributes.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-platform/.../createExposureService.ts | Dropped runtime parameter when calling createCrossThreadEvent. |
| packages/web-platform/.../createCrossThreadEvent.ts | Reads dataset (l-dset) and uniqueId via getAttribute instead of runtime info. |
| packages/web-platform/.../eventFunctions.ts | Updated createCrossThreadEvent calls to match new signature. |
| packages/web-platform/.../elementCreatingFunctions.ts | Removed default lynxDataset initialization from runtime info. |
| packages/web-platform/.../attributeAndPropertyFunctions.ts | Persist dataset to l-dset attribute; removed internal setDatasetAttribute. |
| packages/web-platform/.../ElementThreadElement.ts | Removed lynxDataset property from LynxRuntimeInfo interface. |
| packages/web-platform/web-constants/src/constants.ts | Added lynxDatasetAttribute constant ('l-dset'). |
| .changeset/funny-otters-yawn.md | Added changeset entry for this refactor. |
Comments suppressed due to low confidence (1)
packages/web-platform/web-mainthread-apis/src/elementAPI/attributeAndProperty/attributeAndPropertyFunctions.ts:43
- Add unit tests for the new dataset encoding/decoding logic (
l-dsetanddata-*synchronization) to ensure correct behavior under various edge cases.
export function createAttributeAndPropertyFunctions(
packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts
Outdated
Show resolved
Hide resolved
packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts
Show resolved
Hide resolved
...orm/web-mainthread-apis/src/elementAPI/attributeAndProperty/attributeAndPropertyFunctions.ts
Outdated
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #981 will degrade performances by 14.41%Comparing Summary
Benchmarks breakdown
|
React Example#1121 Bundle Size — 231.7KiB (0%).8bfd370(current) vs 70b82d2 main#1113(baseline) Bundle metrics
|
| Current #1121 |
Baseline #1113 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
141 |
141 |
|
56 |
56 |
|
46.17% |
46.17% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #1121 |
Baseline #1113 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
85.94KiB |
85.94KiB |
Bundle analysis report Branch PupilTong:p/hw/dataset Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#1109 Bundle Size — 254.36KiB (+0.07%).8bfd370(current) vs 70b82d2 main#1101(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch PupilTong:p/hw/dataset Project dashboard Generated by RelativeCI Documentation Report issue |
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.
Pull Request Overview
This PR refactors dataset handling by moving from runtime-stored objects to attribute-based storage (l-dset), ensuring data persistence across render boundaries.
- Migrate dataset reads/writes to use
lynxDatasetAttributewithencodeURIComponent(JSON.stringify(...)) - Remove
lynxDatasetfromLynxRuntimeInfoand all runtime maps - Update event creation to parse dataset from attributes and drop the
runtimeparameter
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| createExposureService.ts | Remove runtime arg when calling createCrossThreadEvent |
| createCrossThreadEvent.ts | Read/write dataset from l-dset attribute instead of runtime |
| eventFunctions.ts | Update createCrossThreadEvent calls to new signature |
| elementCreatingFunctions.ts | Remove runtime dataset initialization |
| attributeAndPropertyFunctions.ts | Implement attribute-based dataset setters/getters |
| ElementThreadElement.ts | Remove lynxDataset from LynxRuntimeInfo interface |
| constants.ts | Add lynxDatasetAttribute constant |
| .changeset/funny-otters-yawn.md | Document dataset attribute refactor |
...orm/web-mainthread-apis/src/elementAPI/attributeAndProperty/attributeAndPropertyFunctions.ts
Show resolved
Hide resolved
packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts
Show resolved
Hide resolved
...orm/web-mainthread-apis/src/elementAPI/attributeAndProperty/attributeAndPropertyFunctions.ts
Outdated
Show resolved
Hide resolved
packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts
Outdated
Show resolved
Hide 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.
Pull Request Overview
This PR refactors how dataset information is stored and retrieved from DOM elements by encoding the entire dataset as a URI‐encoded JSON string using a new attribute. Key changes include:
- Removing the use of in-memory runtime properties for dataset management in favor of using a dedicated attribute (lynxDatasetAttribute).
- Updating functions throughout the codebase (createExposureService, createCrossThreadEvent, elementCreatingFunctions, and attributeAndPropertyFunctions) to align with the new dataset saving strategy.
- Removing the lynxDataset property from runtime information and introducing its handling via DOM attributes.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/web-platform/web-mainthread-apis/src/utils/createExposureService.ts | Removed the runtime parameter in the createCrossThreadEvent call. |
| packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts | Eliminated runtime-based dataset access and now reads dataset directly from attributes. |
| packages/web-platform/web-mainthread-apis/src/elementAPI/elementCreating/elementCreatingFunctions.ts | Removed initialization of the lynxDataset property from runtimeInfo. |
| packages/web-platform/web-mainthread-apis/src/elementAPI/attributeAndProperty/attributeAndPropertyFunctions.ts | Updated dataset-setting logic to encode the dataset and to set individual data- attributes. |
| packages/web-platform/web-constants/src/constants.ts | Added lynxDatasetAttribute constant. |
| .changeset/funny-otters-yawn.md | Updated changeset with corresponding version bumps and summary. |
Comments suppressed due to low confidence (1)
packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts:92
- Direct conversion with Number() might result in NaN if the attribute is missing or malformed. Consider providing a fallback or validating the attribute's presence before conversion.
uniqueId: Number(currentTargetElement.getAttribute?.(lynxUniqueIdAttribute)),
...orm/web-mainthread-apis/src/elementAPI/attributeAndProperty/attributeAndPropertyFunctions.ts
Show resolved
Hide resolved
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - Support for locating errors in the source code directly on the device when exceptions occur when using MTS. ([#1019](#1019)) This requires Lynx engine v3.4 or later. - Fix the "main-thread.js exception: ReferenceError: `__webpack_require__` is not defined" error in HMR. ([#985](#985)) This error occurred when setting `output.iife: true`, which is the default value in `@lynx-js/rspeedy` v0.9.8. ## @lynx-js/[email protected] ### Patch Changes - Set `optimization.emitOnErrors` when `DEBUG` is enabled. ([#1000](#1000)) This is useful for debugging PrimJS Syntax error. ## @lynx-js/[email protected] ### Patch Changes - Better [zustand](https://github.com/pmndrs/zustand) support by creating an alias for `use-sync-external-store`. ([#980](#980)) See [#893](#893) for more details. - Updated dependencies \[[`acc0d80`](acc0d80)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support Windows. ([#1007](#1007)) ## @lynx-js/[email protected] ### Patch Changes - feat: add sheet.insertRule support ([#1026](#1026)) - refactor: implement mts apis in closure pattern ([#1004](#1004)) ## @lynx-js/[email protected] ### Patch Changes - refactor: move some internal status to dom's attribute ([#945](#945)) It's essential for SSR - fix: target.id is undefined ([#1016](#1016)) - feat: add new pageConfig configuration: enableJSDataProcessor ([#886](#886)) - refactor: move component config info to attribute ([#984](#984)) - refactor: save dataset on an attribute ([#981](#981)) On lynx, the `data-*` attributes have different behaviors than the HTMLElement has. The dataset will be treated as properties, the key will not be applied the camel-case <-> hyphenate name transformation. Before this commit we use it as a runtime data, but after this commit we will use encodeURI(JSON.stringify(dataset)) to encode it as a string. - refactor: create elements of `elementToRuntimeInfoMap` on demand ([#986](#986)) - refactor: implement mts apis in closure pattern ([#1004](#1004)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: move some internal status to dom's attribute ([#945](#945)) It's essential for SSR - refactor: avoid to create many style element for cssog ([#1026](#1026)) - refactor: move component config info to attribute ([#984](#984)) - fix: ensure render starts after dom connected ([#1020](#1020)) - refactor: save dataset on an attribute ([#981](#981)) On lynx, the `data-*` attributes have different behaviors than the HTMLElement has. The dataset will be treated as properties, the key will not be applied the camel-case <-> hyphenate name transformation. Before this commit we use it as a runtime data, but after this commit we will use encodeURI(JSON.stringify(dataset)) to encode it as a string. - refactor: implement mts apis in closure pattern ([#1004](#1004)) - Updated dependencies \[[`70b82d2`](70b82d2), [`5651e24`](5651e24), [`9499ea9`](9499ea9), [`50f0193`](50f0193), [`57bf0ef`](57bf0ef), [`5651e24`](5651e24), [`0525fbf`](0525fbf), [`b6b87fd`](b6b87fd), [`c014327`](c014327)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: move some internal status to dom's attribute ([#945](#945)) It's essential for SSR - refactor: move component config info to attribute ([#984](#984)) - refactor: save dataset on an attribute ([#981](#981)) On lynx, the `data-*` attributes have different behaviors than the HTMLElement has. The dataset will be treated as properties, the key will not be applied the camel-case <-> hyphenate name transformation. Before this commit we use it as a runtime data, but after this commit we will use encodeURI(JSON.stringify(dataset)) to encode it as a string. - fix: dump encode data in comment ([#989](#989)) ## @lynx-js/[email protected] ### Patch Changes - feat: x-input && x-textarea add new method: `getValue`, which returns the value of the input element, selectionStart and selectEnd when success. ([#982](#982)) - feat: x-input and x-textarea bindinput event return structures add `selectionStart`, `selectionEnd`, and `textLength`, `textLength` are marked as @deprecated ([#996](#996)) - feat: x-input and x-textarea support bindselection event, the returned type structure is `{ selectionStart: number; selectionEnd: number }`. ([#990](#990)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: move some internal status to dom's attribute ([#945](#945)) It's essential for SSR - refactor: avoid to create many style element for cssog ([#1026](#1026)) - fix: target.id is undefined ([#1016](#1016)) - feat: add new pageConfig configuration: enableJSDataProcessor ([#886](#886)) - refactor: move component config info to attribute ([#984](#984)) - refactor: save dataset on an attribute ([#981](#981)) On lynx, the `data-*` attributes have different behaviors than the HTMLElement has. The dataset will be treated as properties, the key will not be applied the camel-case <-> hyphenate name transformation. Before this commit we use it as a runtime data, but after this commit we will use encodeURI(JSON.stringify(dataset)) to encode it as a string. - refactor: create elements of `elementToRuntimeInfoMap` on demand ([#986](#986)) - refactor: implement mts apis in closure pattern ([#1004](#1004)) - Updated dependencies \[[`70b82d2`](70b82d2), [`9499ea9`](9499ea9), [`50f0193`](50f0193), [`57bf0ef`](57bf0ef), [`0525fbf`](0525fbf), [`b6b87fd`](b6b87fd), [`c014327`](c014327)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: implement mts apis in closure pattern ([#1004](#1004)) - Updated dependencies \[[`70b82d2`](70b82d2), [`5651e24`](5651e24), [`9499ea9`](9499ea9), [`50f0193`](50f0193), [`57bf0ef`](57bf0ef), [`5651e24`](5651e24), [`0525fbf`](0525fbf), [`b6b87fd`](b6b87fd), [`c014327`](c014327)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix `requestAnimationFrame` is not working. ([#1021](#1021)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`ccb4254`](ccb4254)]: - @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
On lynx, the
data-*attributes have different behaviors than the HTMLElement has.The dataset will be treated as properties, the key will not be applied the camel-case <-> hyphenate name transformation.
Before this commit we use it as a runtime data, but after this commit we will use encodeURI(JSON.stringify(dataset)) to encode it as a string.
Implement SSR for Web #52