-
Notifications
You must be signed in to change notification settings - Fork 50
Breaking change: rename "channels" to "components" #454
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
🦋 Changeset detectedLatest commit: 98c375e The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
Deploying terrazzo with
|
| Latest commit: |
98c375e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ad183821.terrazzo.pages.dev |
| Branch Preview URL: | https://update-color.terrazzo.pages.dev |
| case 'lab': { | ||
| await labCmd({ config, configPath, flags, logger }); | ||
| try { | ||
| require.resolve('@terrazzo/token-lab'); |
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.
cc @lilnasy not a perfect solution, but manually requiring folks install this is a short-term solution to breaking the circular dep chain that prevents the CLI from being tested reliably, or having to pull from npm. Having to pull from published packages in the test suite could cause some regressions, and would limit us in what we’re able to test.
But once users install this I think the circular dep may still be there, so I’ll come up with another solution as this gets closer to a stable release (and we’ll celebrate and have lots of fanfare etc etc). For now, this just unblocks the CLI from releasing / auto-installing @terrazzo/token-lab for everyone
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.
This is great. Until stable release, whatever allows progress is a good solution.
| }, | ||
| "scripts": { | ||
| "build": "tsc -p tsconfig.build.json", | ||
| "build": "tsc -p tsconfig.build.json && pnpm run build:lab", |
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.
Test that the lab can build properly
| @@ -1,4 +1,4 @@ | |||
| import '@terrazzo/tokens/dist/index.css'; | |||
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.
@lilnasy This removes one circular dep, but not all.
Between @terrazzo/fonts, @terrazzo/tiles, @terrazzo/tokens, @terrazzo/react-color-picker and @terrazzo/use-color, none of those packages really matter; it’s not important to me to continue publishing those and keeping them separate. The token lab is far more important, and if we wanted to, we could just shove everything React-related in here; that’d be fine. Users could even just import React components from the token-lab, too.
In the future, publishing @terrazzo/react-color-picker would be nice, but still less important than the token lab. Short-term we could just squash all packages into Lab and worry about that later (and we don’t have to use the Terrazzo CLI in the token lab for that fact either—we could just manually style everything who cares).
Changes
The in-progress, updated DTCG color format will use
componentsrather thanchannels. Still allow both for the meantime, but provide a warning for folks for when it’s updatedHow to Review
componentsandchannelsso this is somewhat backwards-compatible