Skip to content
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

token json を結合して color や数値をとれるトークンオブジェクトを作成する #639

Merged
merged 16 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,13 @@ const config = [
},
{
files: [
'**/*.test.ts',
'**/*.test.tsx',
'**/*.test.{ts,tsx}',
'**/__tests__/**',
'**/*.config.*',
'*.config.*',
'**/*.story.*',
'.storybook/**',
'**/*.bench.{ts,tsx}',
],
languageOptions: {
globals: {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"@types/eslint__js": "^8.42.3",
"@types/jest-image-snapshot": "^6.4.0",
"@types/jest-specific-snapshot": "^0.5.9",
"@types/node": "^17.0.13",
"@types/node": "^18.19.59",
"@types/prettier": "^2.4.3",
"@types/styled-components": "^5.1.21",
"@types/webpack": "^5.28.0",
Expand Down
128 changes: 128 additions & 0 deletions packages/theme/cli/token-object.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/* eslint-disable no-console */
import { readFileSync, mkdirSync, writeFileSync } from 'node:fs'
import path from 'node:path'
import { createCSSTokenObject, createTokenObject } from '../src/token-object'
import type { TokenDictionary } from '../src/token-object/types'
import { camelCaseKeys } from '../src/token-object'

type ValueStyle = 'cssVariable'
type KeyStyle = 'camelCase'

type Config = {
tokenFile: string
baseFile: string
outputFile: string
keyStyle?: KeyStyle
valueStyle?: ValueStyle
}

const configurations: Config[] = [
{
tokenFile: './src/json/pixiv-light.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/pixiv-light.json',
keyStyle: undefined,
valueStyle: undefined,
},
{
tokenFile: './src/json/pixiv-dark.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/pixiv-dark.json',
keyStyle: undefined,
valueStyle: undefined,
},

{
tokenFile: './src/json/pixiv-light.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/camel/pixiv-light.json',
keyStyle: 'camelCase',
valueStyle: undefined,
},
{
tokenFile: './src/json/pixiv-dark.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/camel/pixiv-dark.json',
keyStyle: 'camelCase',
valueStyle: undefined,
},

{
tokenFile: './src/json/pixiv-light.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/css-variables/pixiv-light.json',
keyStyle: undefined,
valueStyle: 'cssVariable',
},
{
tokenFile: './src/json/pixiv-dark.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/css-variables/pixiv-dark.json',
keyStyle: undefined,
valueStyle: 'cssVariable',
},

{
tokenFile: './src/json/pixiv-light.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/css-variables/camel/pixiv-light.json',
keyStyle: 'camelCase',
valueStyle: 'cssVariable',
},
{
tokenFile: './src/json/pixiv-dark.json',
baseFile: './src/json/base.json',
outputFile: './dist/tokens/css-variables/camel/pixiv-dark.json',
keyStyle: 'camelCase',
valueStyle: 'cssVariable',
},
]

for (const {
tokenFile,
baseFile,
outputFile,
keyStyle,
valueStyle,
} of configurations) {
const baseJson = JSON.parse(
readFileSync(path.resolve(baseFile), 'utf8')
) as TokenDictionary
Comment on lines +88 to +90
Copy link
Contributor

@yue4u yue4u Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configurationごとreadFile + JSON.parseする必要がないのと、このくらいな量ならasync apiのほうがいいかもしれないですが、ここでそこまでパフォーマンス求めないので一旦これでも大丈夫です

time yarn build 
> yarn build  5.07s user 0.49s system 152% cpu 3.645 total


const createToken = <T extends TokenDictionary>(
value: T
): Record<string, unknown> => {
switch (valueStyle) {
case 'cssVariable': {
return createCSSTokenObject(value, (x) => `charcoal-${x}`)
}
default: {
return createTokenObject(value, baseJson)
}
}
}
const transformKey = <T extends Record<string, unknown>>(
value: T
): Record<string, unknown> => {
switch (keyStyle) {
case 'camelCase': {
return camelCaseKeys(value)
}
default: {
return value
}
}
}

const tokenJson = JSON.parse(
readFileSync(path.resolve(tokenFile), 'utf8')
) as TokenDictionary
const tokenObject = transformKey(createToken(tokenJson))

mkdirSync(path.dirname(outputFile), { recursive: true })
writeFileSync(path.resolve(outputFile), JSON.stringify(tokenObject, null, 2))

console.log(
`Generated ${outputFile} with keyStyle ${keyStyle} and valueStyle ${valueStyle}.`
)
}
46 changes: 38 additions & 8 deletions packages/theme/package.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsonをexportsに足す必要がありそうです

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/pixiv/charcoal/blob/feta/create-styled-tokens/packages/theme/package.json#L21
dist が files に入っていて src/css と src/json を dist に copy するようになってるのでそこは問題ないかなという感じです

Copy link
Contributor

@yue4u yue4u Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いいえ、distにあっても"exports"に足さないとbundlerによってimportすることを許されない気がします。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

スクリーンショット 2024-10-25 11 30 55

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TSからは補完されないし型がない状態

スクリーンショット 2024-10-25 11 28 58

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

以下を足すと少し良くなります

    "./json/*": "./dist/json/*",
    "./css/*": "./dist/css/*"
スクリーンショット 2024-10-25 11 39 24 スクリーンショット 2024-10-25 11 40 41

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いいえ、distにあっても"exports"に足さないとbundlerによってimportすることを許されない気がします。

勘違いしていました。そうでしたね。修正します

Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,69 @@
"license": "Apache-2.0",
"main": "./dist/index.cjs.js",
"module": "./dist/index.esm.js",
"types": "./dist/index.d.ts",
"exports": {
"types": "./dist/index.d.ts",
"require": "./dist/index.cjs.js",
"import": "./dist/index.esm.js",
"default": "./dist/index.esm.js"
".": {
"types": "./dist/index.d.ts",
"require": "./dist/index.cjs.js",
"import": "./dist/index.esm.js",
"default": "./dist/index.esm.js"
},
"./token-object": {
"types": "./dist/token-object/index.d.ts",
"require": "./dist/token-object/index.cjs.js",
"import": "./dist/token-object/index.esm.js",
"default": "./dist/token-object/index.esm.js"
},
"./css/*": "./dist/css/*",
"./tokens/*": "./dist/tokens/*"
},
"typesVersions": {
"*": {
"token-object": [
"./dist/token-object/index.d.ts"
],
"tokens/*": [
"./dist/tokens/*"
],
"css/*": [
"./dist/css/*"
]
}
},
"types": "./dist/index.d.ts",
"sideEffects": [
"*.css"
],
"scripts": {
"build": "npm-run-all --print-label --parallel 'build:*' --sequential serialize",
"build": "npm-run-all --print-label -s 'build:*' --sequential serialize",
"build:bundle": "FORCE_COLOR=1 tsup",
"build:dts": "tsc --project tsconfig.build.json --pretty --emitDeclarationOnly",
"build:copy-css": "cpx 'src/css/**/*.css' dist/css && cpx 'src/json/**/*.json' dist/json",
"build:copy-css": "cpx --clean 'src/css/**/*.{css,d.ts}' dist/css",
"build:token-object": "tsx cli/token-object.ts",
"serialize": "node cli/index.js",
"typecheck": "run-p --print-label 'typecheck:*'",
"typecheck:main": "tsc --project tsconfig.build.json --pretty --noEmit",
"typecheck:cli": "tsc --project cli/tsconfig.build.json --noEmit",
"clean": "rimraf dist .tsbuildinfo",
"test": "vitest run --passWithNoTests"
"test": "vitest run --passWithNoTests",
"bench": "vitest bench --passWithNoTests"
},
"devDependencies": {
"@types/css": "^0.0.38",
"cpx": "^1.5.0",
"css": "^3.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

気づいてなかったでした。
https://www.npmjs.com/package/css メンテされていなさそうでpostcssかlightningcssに置き換えたらいいかもしれないですね

"npm-run-all": "^4.1.5",
"rimraf": "^3.0.2",
"tsup": "^6.5.0",
"tsx": "^4.19.1",
"typescript": "^4.9.5",
"vitest": "^2.0.2"
},
"dependencies": {
"@charcoal-ui/foundation": "^4.0.0-beta.15",
"@charcoal-ui/utils": "^4.0.0-beta.15",
"change-case-all": "^2.1.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change-case が esm only で commonjs build できないので対応しているほうを導入
github の star は少ないが @graphql/xxx にも使用されているので問題なしと判断

https://www.npmjs.com/package/change-case-all?activeTab=dependents

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

割とesmがblockingになるケース最近あんまりない気がするけど、一旦change-case-allにしても大丈夫です。change-caseの若干(500Bくらい)小さいです

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使う側はそうなんですが、@charcoal/theme 自体が dual pacakge build しているので create-token-object を package として export するなら esm, commonjs 対応の依存が必要になってくるというやつでした

"deepmerge": "^4.3.1",
Copy link
Contributor

@yue4u yue4u Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

二つのライブラリーもruntimeに入れるくらいなら事前に作ったものと実際比較してみたいです
(現在両方+2kBくらい)

また、dependenciesにあるとビルドされても使う側でもう一度installされっるのでdevDependenciesに移動したいかもしれないです

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

メモ: こういう形にすると一つ4KB gzip (12KB before)
スクリーンショット 2024-10-23 20 43 56

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/pixiv/charcoal/blob/feta/create-styled-tokens/packages/theme/src/token-object/__snapshots__/token-object.test.ts.snap

css variables のものと camelcase のもの, json の key そのままのものを複数用意しました。すべて dist/json に吐き出されるようにしています

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらのdepsもうdevdependenciesに移動していい気がしました

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subpath で export している以上 dependencies には必要かなと思っていたんですが、devDependencies にして subpath による export 切っちゃったほうが良いですかね?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ビルド時bundleされていれば不要と言おうとしていたがdepsはbundleされていないようですね。一旦現状でも大丈夫です

"polished": "^4.1.4"
},
"files": [
Expand Down
1 change: 1 addition & 0 deletions packages/theme/src/css/_variables_dark.css.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module "*.css" { }
1 change: 1 addition & 0 deletions packages/theme/src/css/_variables_light.css.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module "*.css" { }
Loading
Loading