feat: Create utils package to share logic across runtime and bundler #345
Conversation
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
brijeshb42
force-pushed
the
v1/utils-package
branch
from
b30ef1e to
4003aa6
Compare
brijeshb42
force-pushed
the
v1/utils-package
branch
6 times, most recently
from
e1c0ebc to
903cc78
Compare
mnajdova
reviewed
Dec 20, 2024
| @@ -1,8 +1,7 @@ | |||
| { | |||
| "extends": "../../tsconfig.json", | |||
| "compilerOptions": { | |||
| "resolveJsonModule": true, | |||
| "target": "ES2015" | |||
| "skipLibCheck": true | |||
There was a problem hiding this comment.
Without it, it was typechecking for stuff inside unplugin package that does not affect our package.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(1,31): error TS2307: Cannot find module '@farmfe/core' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(2,46): error TS2307: Cannot find module '@farmfe/core' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(3,47): error TS2307: Cannot find module '@rspack/core/dist/config/zod' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(7,45): error TS2307: Cannot find module 'rolldown/dist/types/plugin' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(14,126): error TS2307: Cannot find module '@rspack/core' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(15,66): error TS2307: Cannot find module '@rspack/core' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(19,36): error TS2307: Cannot find module 'rolldown' or its corresponding type declarations.
../../node_modules/.pnpm/[email protected][email protected]/node_modules/unplugin/dist/index.d.mts(20,42): error TS2307: Cannot find module 'rolldown' or its corresponding type declarations.
ELIFECYCLE Command failed with exit code 2.
This check can be safely ignored since at the moment we don't care about farm/rolldown/rspack etc.
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@pigment-css/theme", | |||
| "version": "0.0.27", | |||
| "version": "0.0.28", | |||
There was a problem hiding this comment.
We can bump the version during the release process.
| "module": "build/index.mjs", | ||
| "types": "build/index.d.ts", | ||
| "author": "MUI Team", | ||
| "description": "Utilities to be used internally across Pigment CSS libraries.", |
There was a problem hiding this comment.
Suggested change
| "description": "Utilities to be used internally across Pigment CSS libraries.", | |
| "description": "Utilities to be used internally across different Pigment CSS packages.", |
nit
| /** | ||
| * Parses array expression string to AST for locating sourcemap points. | ||
| */ | ||
| export function parseArray( |
There was a problem hiding this comment.
Should we rename the file to parseArray.ts, or at least to parseExpressions.ts (plural) if we plan to add more things.
| >; | ||
| defaultVariants?: Record<string, string | number>; | ||
| }; | ||
| type BaseStyleObject = ExtendedStyleObj & Record<string, string | object>; |
There was a problem hiding this comment.
Don't we miss here the CSS template string type?
| }, | ||
| }, | ||
| variables: { | ||
| '--var-1': [style[`.cls1`].color, 0], |
There was a problem hiding this comment.
Where would this style[`.cls1`].color be defined?
Comment on lines
+13
to
+14
| $$hello: 'world', | ||
| $hello1: 'world', |
There was a problem hiding this comment.
Suggested change
| $$hello: 'world', | |
| $hello1: 'world', | |
| $$hello: 'world', | |
| $hello1: 'world', |
What's the difference betweem $ and $$?
brijeshb42
force-pushed
the
v1/utils-package
branch
from
903cc78 to
1a1a33b
Compare
brijeshb42
force-pushed
the
v1/utils-package
branch
from
1a1a33b to
95d0598
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…packages
For reference, here's the next PR that uses this package.
This PR has the following utils -
$$valuekeys in the css object to convert to css variables---valuevariants,compoundVariantsanddefaultVariantskeysvalueToLiteral- Copied over from@pigment-css/reactto convert JS values to equivalent AST.Also, fixes tsconfig to used the latest config from core and updated the script to actually run typecheck in the CI which was not happening right now.