feat: DesignToken refactor for FF3 #6171

nicholasrice · 2022-06-29T17:11:51Z

Pull Request

📖 Description

This PR refactors the FF DesignToken feature to align with #6134. The notable changes follow:

Refactor DesignToken to a core implementation that is isomorphic

The entire feature has been refactored and built on top of a core infrastructure that does not leverage any DOM APIs. This means the core can be used to build implementations that work in any JavaScript environment, including plugins for design software such as Figma.

Restrict mutation operations to FASTElement instances

getting, setting, and deleting operations for DesignToken now require a FASTElement instance to operate on instead of any HTMLElement reference. This was done to patch up some odd edge case bugs and to ensure DesignToken behavior between SSR and CSR would be consistent.

DesignToken and CSSDesignToken classes are now exported

Two new exports, DesignToken and CSSDesignToken replace the original DesignToken factory and configuration object. Additoinally, function type DesignToken instances can now be created, where before they were restricted:

DesignToken.create<() => boolean>("my-function-design-token")

Adjust derived values to receive a token resolver argument

Derived values are provided 'resolver' function, where before they were provided a target HTMLElement. This pattern is easier to understand (I think it was always a little confusing why an HTML element was provided) and also remove HTMLElement as a core dependency of the system, allowing it to work outside the browser. For example, assigning a derived value that uses a token now looks like this:

someToken.withDefault((resolve) => resolve(someOtherToken) * 2);

🎫 Issues

Closes #6134, #5903, #5902

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

I have included a change request file using $ yarn change
I have added tests for my changes.
I have tested my changes.
I have updated the project documentation to reflect my changes.
I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

I have added a new component
I have modified an existing component
I have updated the definition file
I have updated the configuration file

⏭ Next Steps

KingOfTac · 2022-08-03T17:25:27Z

Does this fix interpolating tokens in css templates not working? That is something that I noticed in alpha.5 after DesignSystem was removed.

…itself

…tion

…s://github.com/microsoft/fast into users/nirice/design-token-refactor-exploration

change/@microsoft-adaptive-ui-881bac8a-4800-4331-9138-0c658998c854.json

…c854.json

change/@microsoft-fast-element-2a4431cf-f1de-44ed-bae4-392f10b74b86.json

…74b86.json

change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc37d60605.json

…37d60605.json

… script

…tion

EisenbergEffect · 2022-08-08T19:35:44Z

packages/web-components/fast-foundation/src/design-token/core/design-token-node.ts

+    }
+
+    public get children(): DesignTokenNode[] {
+        return Array.from(this._children);


Should it return a new array every time?

I think so, but I'm open to suggestions. The children are stored in a Set privately to make identity lookups easier, but I don't want that set mutable publicly so I convert it to an array for public consumption.

EisenbergEffect · 2022-08-08T19:50:06Z

packages/web-components/fast-foundation/src/design-token/core/design-token-node.ts

+                    this.children[i].dispatch(records[j]);
+                }
+            }
+        }


Not sure if it makes sense, but would there be an efficiencies gained by instead sending the list of records to child instead of using nested for loops to send the individual records? Maybe they just have to loop internally anyway and so there's no advantage. Just thought I'd check.

Follow on question, is this the dispatch an array improvement slated for a later date?

Yeah I touched on it in our sync, but I think we can see some improvement by reconciling all changes for each node and dispatching a set of mutations instead of dispatching one mutation at a time. I'll file a tracking issue.

change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc37d60605.json

janechu · 2022-08-08T22:52:11Z

packages/web-components/fast-foundation/package.json

@@ -27,6 +27,10 @@
      "default": "./dist/esm/index.js"
    },
    "./custom-elements.json": "./dist/custom-elements.json",
+    "./design-token-core": {


Should we export also from the exports.js file in the design-token folder as well as the core export? Or package them together so everything exports from the ./dist/esm/design-tokens/exports.js file?

I want to keep them separate. To start, there are some exports with the same name so that would make conflicts or force us to change the name. Additionally, I think most folks don't need to use the core infrastructure, so keeping that separate from the primary package makes sense to me.

packages/web-components/fast-foundation/src/design-token/fast-design-token.ts

janechu · 2022-08-08T23:03:05Z

packages/web-components/fast-foundation/src/design-token/fast-design-token.ts

+    }
+
+    constructor(public readonly target: FASTElement) {
+        super();


Do we need this?

Yes, it declares the target property.

janechu · 2022-08-08T23:19:42Z

packages/web-components/fast-foundation/src/design-token/core/design-token-node.ts

+                    this.children[i].dispatch(records[j]);
+                }
+            }
+        }


Follow on question, is this the dispatch an array improvement slated for a later date?

…37d60605.json Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com>

…design-token.ts Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com>

…tion

* adding interfaces for new API structure * adding tests for appendChild / removeChild * add a stub implementation of DesignToken * adding minimal design token interface * save-progress * adding notification logic * re-organize notification propagation logic * adding getTokenValue and tests * add notification behavior to appndChild * refactor to use string id * rename method to retrieve all tokens for a node * refactor to fix chai bug and issues uncovered when avoiding chai issue * refactor a bit to centralize notifier for tokens on the token object itself * adding new test for no notification when assigned the same value * fixing test * adding subscription test * implement isAssigned * progress on derived tokens * implement mechanism to notify of static token changes * make derived token assignments notify down the token tree * adding and adapting test-cases from original DesignToken implementation * re-organization of DesignTokenNode * adding observable map to notify for derived values * naieve implementation of reflecting upstream derived tokens * re-oragnize feature and add exports to facilitate testing * implement circular reference resolution and made a few perf changes * adding another circular reference test * adding deleteTokenValue API and privatizing isDerivedFor * adding subscription tests * update test names * further test clean up * more tests and slight performance improvement * method organization * remove observableMap implementation to provide better control over token notification, adds a few tests * implement more specific notification behavior * fixing test so it passes w/ new notification behavior * adding test package * adds behavior to handle observable value updates * adding additional observable tests * refactor token notification to contain mutation type data * refactor notifications and fix token deletion tests * adding notification behavior to appending a node * update readme with perf improvement notes * in-progress changes for node notification on removal of token and refactoring notification to be single-pass * refactor notification behavior and fix all test * batch notifications so the tree is reconciled before subscribers are notified of a change * implement DesignToken over top of DesignTokenNode, CSS reflection * isolate design token core from FAST design token implementation * implement root registration * Ensure nodes for detached elements can access the defualt node * passing all tests * fixing tests * fixing import * a bit of cleanup * refactor type constraints that opens up exporting the token classes instead of an interface/const * re-organizing files * re-organize assets, exports, and files. delete legacy design-token * fix bug in recursive reconciliation of tokens * update adaptive UI package * consolidate notification queue * add children notifier function * make CSSDesignToken a real CSSDirective * fixing circular dependency notification bug * fixing bug in CSS design token css value calculation * implement observable derived value structure * refactor to not itterate through all derived values during update * improving performance of derived tokens * fixing example * removing un-necessary method * adding css directive test * create tokens with unique name * lazily create subscriber set for tokens * implement resolution strategy * removing debugging spies * fixing bug in observable bindings where the watcher was not reset if the binding threw * fixing memory leak caused by observable subscribers * cleaning up design-token-node tests * removing tests for scenario that is no longer supported * adding method comments * remove test package * fixing after rebase * Change files * Update change/@microsoft-adaptive-ui-881bac8a-4800-4331-9138-0c658998c854.json * Update change/@microsoft-fast-element-2a4431cf-f1de-44ed-bae4-392f10b74b86.json * Update change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc37d60605.json * removing test site from workspaces * removing readme that is getting processesd by custom element manifest script * adding API documentation * removing notes file * fixing build * Update change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc37d60605.json Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com> * Update packages/web-components/fast-foundation/src/design-token/fast-design-token.ts Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com> Co-authored-by: nicholasrice <nicholasrice@users.noreply.github.com> Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com>

nicholasrice force-pushed the users/nirice/design-token-refactor-exploration branch 2 times, most recently from ecdf0df to 8096ceb Compare July 5, 2022 21:08

nicholasrice force-pushed the users/nirice/design-token-refactor-exploration branch 4 times, most recently from 0da7115 to f31204d Compare July 15, 2022 19:09

nicholasrice force-pushed the users/nirice/design-token-refactor-exploration branch from c1456b3 to 4165547 Compare July 18, 2022 18:24

nicholasrice force-pushed the users/nirice/design-token-refactor-exploration branch from b1f0b41 to 28bfdc5 Compare July 27, 2022 20:22

nicholasrice added 21 commits August 3, 2022 11:14

adding interfaces for new API structure

7128199

adding tests for appendChild / removeChild

9451732

add a stub implementation of DesignToken

469b1d6

adding minimal design token interface

6452e01

save-progress

c5d27e9

adding notification logic

5d7e8cb

re-organize notification propagation logic

18f6ba2

adding getTokenValue and tests

0dba736

add notification behavior to appndChild

56eb369

refactor to use string id

1a3181b

rename method to retrieve all tokens for a node

65f70bd

refactor to fix chai bug and issues uncovered when avoiding chai issue

afc9467

refactor a bit to centralize notifier for tokens on the token object …

dcfa22f

…itself

adding new test for no notification when assigned the same value

5450c15

fixing test

64a4988

adding subscription test

43ea386

implement isAssigned

610a61e

progress on derived tokens

7348d91

implement mechanism to notify of static token changes

f83ad12

make derived token assignments notify down the token tree

6000841

adding and adapting test-cases from original DesignToken implementation

7d072ab

nicholasrice requested a review from Falkicon as a code owner August 3, 2022 19:02

nicholasrice and others added 3 commits August 3, 2022 12:03

Merge branch 'master' into users/nirice/design-token-refactor-explora…

dca161c

…tion

Change files

81da3c7

Merge branch 'users/nirice/design-token-refactor-exploration' of http…

6b646dc

…s://github.com/microsoft/fast into users/nirice/design-token-refactor-exploration

nicholasrice commented Aug 3, 2022

View reviewed changes

change/@microsoft-adaptive-ui-881bac8a-4800-4331-9138-0c658998c854.json Outdated Show resolved Hide resolved

Update change/@microsoft-adaptive-ui-881bac8a-4800-4331-9138-0c658998…

f8dc19d

…c854.json

nicholasrice commented Aug 3, 2022

View reviewed changes

change/@microsoft-fast-element-2a4431cf-f1de-44ed-bae4-392f10b74b86.json Outdated Show resolved Hide resolved

Update change/@microsoft-fast-element-2a4431cf-f1de-44ed-bae4-392f10b…

a9d2d15

…74b86.json

nicholasrice commented Aug 3, 2022

View reviewed changes

change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc37d60605.json Outdated Show resolved Hide resolved

nicholasrice and others added 4 commits August 3, 2022 13:55

Update change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc…

edeb884

…37d60605.json

removing test site from workspaces

baea9b5

removing readme that is getting processesd by custom element manifest…

63ea814

… script

adding API documentation

e1ecf34

nicholasrice force-pushed the users/nirice/design-token-refactor-exploration branch from 2653215 to e1ecf34 Compare August 3, 2022 21:18

nicholasrice and others added 3 commits August 3, 2022 14:28

removing notes file

4a9a81a

fixing build

a2f6319

Merge branch 'master' into users/nirice/design-token-refactor-explora…

9d0adfe

…tion

nicholasrice added this to the FAST Foundation 3.0 milestone Aug 5, 2022

This was linked to issues Aug 5, 2022

feat: restrict DesignToken to being set for FASTElement instances only #5902

Closed

feat: add token resolution strategy to DesignToken #5903

Closed

Merge branch 'master' into users/nirice/design-token-refactor-explora…

7e6454b

…tion

EisenbergEffect approved these changes Aug 8, 2022

View reviewed changes

janechu approved these changes Aug 8, 2022

View reviewed changes

nicholasrice and others added 3 commits August 9, 2022 10:10

Update change/@microsoft-fast-foundation-d9cdf375-e6b7-4a6e-87e6-81dc…

19ecfd3

…37d60605.json Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com>

Update packages/web-components/fast-foundation/src/design-token/fast-…

fb343cf

…design-token.ts Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com>

Merge branch 'master' into users/nirice/design-token-refactor-explora…

9bd20b4

…tion

nicholasrice merged commit 218cfc0 into master Aug 9, 2022

nicholasrice deleted the users/nirice/design-token-refactor-exploration branch August 9, 2022 17:35

nicholasrice mentioned this pull request Aug 9, 2022

docs: Update DesignToken documentation to address API changes from #6171 #6273

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: DesignToken refactor for FF3 #6171

feat: DesignToken refactor for FF3 #6171

nicholasrice commented Jun 29, 2022 •

edited

Loading

KingOfTac commented Aug 3, 2022

EisenbergEffect Aug 8, 2022

nicholasrice Aug 9, 2022

EisenbergEffect Aug 8, 2022

janechu Aug 8, 2022

nicholasrice Aug 9, 2022

janechu Aug 8, 2022

nicholasrice Aug 9, 2022

janechu Aug 8, 2022

nicholasrice Aug 9, 2022

janechu Aug 8, 2022

feat: DesignToken refactor for FF3 #6171

feat: DesignToken refactor for FF3 #6171

Conversation

nicholasrice commented Jun 29, 2022 • edited Loading

Pull Request

📖 Description

Refactor DesignToken to a core implementation that is isomorphic

Restrict mutation operations to FASTElement instances

DesignToken and CSSDesignToken classes are now exported

Adjust derived values to receive a token resolver argument

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

Component-specific

⏭ Next Steps

KingOfTac commented Aug 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicholasrice commented Jun 29, 2022 •

edited

Loading