Fix plugin-css, plugin-sass #150
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@cobalt-ui/plugin-css': patch | ||
| --- | ||
|
|
||
| Bugfix: bugs in transform() API introduced in 1.6.0 |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@cobalt-ui/plugin-sass': patch | ||
| '@cobalt-ui/plugin-css': patch | ||
| --- | ||
|
|
||
| Bugfix: fix TypeScript signature for transform() plugin option |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@cobalt-ui/plugin-sass': patch | ||
| '@cobalt-ui/plugin-css': patch | ||
| --- | ||
|
|
||
| Bugfix: mismatched versions of plugin-css and plugin-sass (if either were on `latest`) were broken. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ import type { | |
| } from '@cobalt-ui/core'; | ||
| import {indent, isAlias, kebabinate, FG_YELLOW, RESET} from '@cobalt-ui/utils'; | ||
| import {clampChroma, converter, formatCss, formatHex, formatHex8, formatHsl, formatRgb, parse as parseColor} from 'culori'; | ||
| import {CustomNameGenerator, DASH_PREFIX_RE, defaultNameGenerator, makeNameGenerator} from './utils/generate-token-name.js'; | ||
| import {encode} from './utils/encode.js'; | ||
| import {formatFontNames} from './utils/format-font-names.js'; | ||
| import {isTokenMatch} from './utils/is-token-match.js'; | ||
|
|
@@ -45,7 +45,7 @@ export interface Options { | |
| /** generate wrapper selectors around token modes */ | ||
| modeSelectors?: ModeSelector[] | LegacyModeSelectors; | ||
| /** handle different token types */ | ||
| transform?: <T extends ParsedToken>(token: T, mode?: string) => string | undefined | null; | ||
| /** @deprecated prefix variable names */ | ||
| prefix?: string; | ||
| /** enable P3 color enhancement? (default: true) */ | ||
|
|
@@ -123,9 +123,9 @@ export default function pluginCSS(options?: Options): Plugin { | |
| const selectors: string[] = []; | ||
| const colorFormat = options?.colorFormat ?? 'hex'; | ||
| for (const token of tokens) { | ||
| let value: ReturnType<typeof defaultTransformer> | undefined | null = await options?.transform?.(token); | ||
| if (value === undefined || value === null) { | ||
| value = defaultTransformer(token, {prefix, colorFormat, generateName, tokens}); | ||
| } | ||
| switch (token.$type) { | ||
| case 'link': { | ||
|
|
@@ -182,9 +182,9 @@ export default function pluginCSS(options?: Options): Plugin { | |
| for (const selector of modeSelector.selectors) { | ||
| if (!selectors.includes(selector)) selectors.push(selector); | ||
| if (!modeVals[selector]) modeVals[selector] = {}; | ||
| let modeVal: ReturnType<typeof defaultTransformer> | undefined | null = await options?.transform?.(token, modeSelector.mode); | ||
| if (modeVal === undefined || modeVal === null) { | ||
| modeVal = defaultTransformer(token, {colorFormat, generateName, mode: modeSelector.mode, prefix, tokens}); | ||
| } | ||
| switch (token.$type) { | ||
| case 'link': { | ||
|
|
@@ -354,90 +354,101 @@ export function transformStrokeStyle(value: ParsedStrokeStyleToken['$value']): s | |
|
|
||
| export function defaultTransformer( | ||
| token: ParsedToken, | ||
| { | ||
| colorFormat = 'hex', | ||
| generateName, | ||
| mode, | ||
| prefix, | ||
| tokens, | ||
| }: { | ||
| colorFormat: Options['colorFormat']; | ||
| generateName?: ReturnType<typeof makeNameGenerator>; | ||
| mode?: string; | ||
| prefix?: string; | ||
| tokens?: ParsedToken[]; | ||
| }, | ||
| ): string | number | Record<string, string> { | ||
| switch (token.$type) { | ||
| // base tokens | ||
| case 'color': { | ||
| const {originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal, {prefix, tokens, generateName}); | ||
| } | ||
| return transformColor(originalVal, colorFormat); // note: use original value because it may have been normalized to hex (which matters if it wasn’t in sRGB gamut to begin with) | ||
| } | ||
| case 'dimension': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformDimension(value); | ||
| } | ||
| case 'duration': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformDuration(value); | ||
| } | ||
| case 'font' as 'fontFamily': // @deprecated (but keep support for now) | ||
| case 'fontFamily': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformFontFamily(value); | ||
| } | ||
| case 'fontWeight': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformFontWeight(value); | ||
| } | ||
| case 'cubicBezier': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformCubicBezier(value); | ||
| } | ||
| case 'number': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformNumber(value); | ||
| } | ||
| case 'link': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformLink(value); | ||
| } | ||
| case 'strokeStyle': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (isAlias(originalVal)) { | ||
| return varRef(originalVal as string, {prefix, tokens, generateName}); | ||
| } | ||
| return transformStrokeStyle(value); | ||
| } | ||
| // composite tokens | ||
| case 'border': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (typeof originalVal === 'string') { | ||
| return varRef(originalVal, {prefix, tokens, generateName}); | ||
| } | ||
| const width = isAlias(originalVal.width) ? varRef(originalVal.width, {prefix, tokens, generateName}) : transformDimension(value.width); | ||
| const color = isAlias(originalVal.color) ? varRef(originalVal.color, {prefix, tokens, generateName}) : transformColor(originalVal.color, colorFormat); | ||
| const style = isAlias(originalVal.style) ? varRef(originalVal.style as string, {prefix, tokens, generateName}) : transformStrokeStyle(value.style); | ||
| return `${width} ${style} ${color}`; | ||
| } | ||
| case 'shadow': { | ||
| let {value, originalVal} = getMode(token, mode); | ||
| if (typeof originalVal === 'string') { | ||
| return varRef(originalVal, {prefix, tokens, generateName}); | ||
| } | ||
|
|
||
| // handle backwards compat for previous versions that didn’t always return array | ||
|
|
@@ -448,56 +459,56 @@ export function defaultTransformer( | |
| .map((shadow, i) => { | ||
| const origShadow = originalVal[i]!; | ||
| if (typeof origShadow === 'string') { | ||
| return varRef(origShadow, {prefix, tokens, generateName}); | ||
| } | ||
| const offsetX = isAlias(origShadow.offsetX) ? varRef(origShadow.offsetX, {prefix, tokens, generateName}) : transformDimension(shadow.offsetX); | ||
| const offsetY = isAlias(origShadow.offsetY) ? varRef(origShadow.offsetY, {prefix, tokens, generateName}) : transformDimension(shadow.offsetY); | ||
| const blur = isAlias(origShadow.blur) ? varRef(origShadow.blur, {prefix, tokens, generateName}) : transformDimension(shadow.blur); | ||
| const spread = isAlias(origShadow.spread) ? varRef(origShadow.spread, {prefix, tokens, generateName}) : transformDimension(shadow.spread); | ||
| const color = isAlias(origShadow.color) ? varRef(origShadow.color, {prefix, tokens, generateName}) : transformColor(origShadow.color, colorFormat); | ||
| return `${shadow.inset ? 'inset ' : ''}${offsetX} ${offsetY} ${blur} ${spread} ${color}`; | ||
| }) | ||
| .join(', '); | ||
| } | ||
| case 'gradient': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (typeof originalVal === 'string') { | ||
| return varRef(originalVal, {prefix, tokens, generateName}); | ||
| } | ||
| return value | ||
| .map((gradient, i) => { | ||
| const origGradient = originalVal[i]!; | ||
| if (typeof origGradient === 'string') { | ||
| return varRef(origGradient, {prefix, tokens, generateName}); | ||
| } | ||
| const color = isAlias(origGradient.color) ? varRef(origGradient.color, {prefix, tokens, generateName}) : transformColor(origGradient.color, colorFormat); | ||
| const stop = isAlias(origGradient.position) ? varRef(origGradient.position as any, {prefix, tokens, generateName}) : `${100 * gradient.position}%`; | ||
| return `${color} ${stop}`; | ||
| }) | ||
| .join(', '); | ||
| } | ||
| case 'transition': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (typeof originalVal === 'string') { | ||
| return varRef(originalVal, {prefix, tokens, generateName}); | ||
| } | ||
| const duration = isAlias(originalVal.duration) ? varRef(originalVal.duration, {prefix, tokens, generateName}) : transformDuration(value.duration); | ||
| let delay: string | undefined = undefined; | ||
| if (value.delay) { | ||
| delay = isAlias(originalVal.delay) ? varRef(originalVal.delay, {prefix, tokens, generateName}) : transformDuration(value.delay); | ||
| } | ||
| const timingFunction = isAlias(originalVal.timingFunction) ? varRef(originalVal.timingFunction as any, {prefix, tokens, generateName}) : transformCubicBezier(value.timingFunction); | ||
| return `${duration} ${delay ?? ''} ${timingFunction}`; | ||
| } | ||
| case 'typography': { | ||
| const {value, originalVal} = getMode(token, mode); | ||
| if (typeof originalVal === 'string') { | ||
| return varRef(originalVal, {prefix, tokens, generateName}); | ||
| } | ||
| const output: Record<string, string> = {}; | ||
| for (const [k, v] of Object.entries(value)) { | ||
| const formatter = k === 'fontFamily' ? transformFontFamily : (val: any): string => String(val); | ||
| output[kebabinate(k)] = isAlias((originalVal as any)[k] as any) ? varRef((originalVal as any)[k], {prefix, tokens, generateName}) : formatter(v as any); | ||
| } | ||
| return output; | ||
| } | ||
|
|
@@ -518,25 +529,38 @@ function getMode<T extends {id: string; $value: any; $extensions?: any; _origina | |
| return {value: token.$value, originalVal: token._original.$value}; | ||
| } | ||
|
|
||
| /** | ||
| * Reference an existing CSS var | ||
| * | ||
| */ | ||
| export function varRef( | ||
| id: string, | ||
| options?: { | ||
| prefix?: string; | ||
| suffix?: string; | ||
| // note: the following properties are optional to preserve backwards-compat without a breaking change | ||
| tokens?: ParsedToken[]; | ||
| generateName?: ReturnType<typeof makeNameGenerator>; | ||
| }, | ||
| ): string { | ||
| let refID = id; | ||
| if (isAlias(id)) { | ||
| // unclear if mode is ever appended to id when passed here, but leaving for safety in case | ||
| const [rootID, _mode] = id.substring(1, id.length - 1).split('#'); | ||
| refID = rootID!; | ||
| } | ||
|
|
||
| const token = options?.tokens?.find((t) => t.id === refID); | ||
|
|
||
| if (!token) { | ||
| console.warn(`Tried to reference variable with id: ${refID}, no token found`); // eslint-disable-line no-console | ||
| } | ||
|
|
||
| // suffix is only used internally (one place in plugin-sass), so handle it here rather than clutter the public API in defaultNameGenerator | ||
| const normalizedSuffix = options?.suffix ? `-${options?.suffix.replace(DASH_PREFIX_RE, '')}` : ''; | ||
| const variableId = refID + normalizedSuffix; | ||
|
|
||
| return `var(${options?.generateName?.(variableId, token) ?? defaultNameGenerator(variableId, options?.prefix)})`; | ||
|
There was a problem hiding this comment. So does this mean
...are already accounted for in the There was a problem hiding this comment.
Maybe not “public”, but used in the Sass plugin. And so mismatched versions would throw an error due to the changed signature. It’s a TODO item for cleanup |
||
| } | ||
|
|
||
| /** @deprecated parse legacy modeSelector */ | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| { | ||
| "color": { | ||
| "$type": "color", | ||
| "blue": { | ||
| "0": {"$value": "#ddf4ff"}, | ||
| "1": {"$value": "#b6e3ff"}, | ||
| "2": {"$value": "#80ccff"}, | ||
| "3": {"$value": "#54aeff"}, | ||
| "4": {"$value": "#218bff"}, | ||
| "5": {"$value": "#0969da"}, | ||
| "6": {"$value": "#0550ae"}, | ||
| "7": {"$value": "#033d8b"}, | ||
| "8": {"$value": "#0a3069"}, | ||
| "9": {"$value": "#002155"} | ||
| } | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /** | ||
| * Design Tokens | ||
| * Autogenerated from tokens.json. | ||
| * DO NOT EDIT! | ||
| */ | ||
|
|
||
| :root { | ||
| --color-blue-0: #ddf4ff; | ||
| --color-blue-1: #b6e3ff; | ||
| --color-blue-2: #80ccff; | ||
| --color-blue-3: #54aeff; | ||
| --color-blue-4: #218bff; | ||
| --color-blue-5: #0969db; | ||
| --color-blue-6: #0550ae; | ||
| --color-blue-7: #033d8b; | ||
| --color-blue-8: #0a3069; | ||
| --color-blue-9: #002155; | ||
| } | ||
|
|
||
| @supports (color: color(display-p3 1 1 1)) { | ||
| :root { | ||
| --color-blue-0: color(display-p3 0.8666666666666667 0.9568627450980393 1); | ||
| --color-blue-1: color(display-p3 0.7137254901960784 0.8901960784313725 1); | ||
| --color-blue-2: color(display-p3 0.5019607843137255 0.8 1); | ||
| --color-blue-3: color(display-p3 0.32941176470588235 0.6823529411764706 1); | ||
| --color-blue-4: color(display-p3 0.12941176470588237 0.5450980392156862 1); | ||
| --color-blue-5: color(display-p3 0.03529411764705882 0.4117647058823529 0.8588235294117647); | ||
| --color-blue-6: color(display-p3 0.0196078431372549 0.3137254901960784 0.6823529411764706); | ||
| --color-blue-7: color(display-p3 0.011764705882352941 0.23921568627450981 0.5450980392156862); | ||
| --color-blue-8: color(display-p3 0.0392156862745098 0.18823529411764706 0.4117647058823529); | ||
| --color-blue-9: color(display-p3 0 0.12941176470588237 0.3333333333333333); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Does this mean they will be removed with the
const token = options?.tokens?.find((t) => t.id === refID);find and warn bellow once there is a breaking change?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.
We’re following semver in this package, so there shouldn’t be any breaking changes until
2.0.0.This was just an unintentional internal change that accidentally led to a breaking change. It was triggered by mismatching versions of plugin-css (+/-
1.6.0) and plugin-sass (+/-1.3.2). This change just allows for (hopefully) graceful fallback between the two plugins. The optional chaining method just allows an older version of the plugin to omit that data, and it will, still render a result.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.
Hidden cross-dependencies like this are an antipattern in general, and something I should clean up better. Just at the moment, it felt like saving work / reducing bugs to have plugin-sass rely on plugin-css, but over time they’ve become too tightly-coupled and I think there needs to be a common core they share that allows for safer version drift between the two (irrelevant for consumers; this is just an implementation detail I’m musing on).