[SWC] - Adds rust based replica of babel-plugin and accompanying tests #1843
Conversation
shanshrew
requested review from
JakeLane,
pancaspe87,
liamqma,
dddlr,
kylorhall-atlassian and
danieldelcore
as code owners
|
✅ Deploy Preview for compiled-css-in-js canceled.
|
There was a problem hiding this comment.
To hoist the comment—this is an "experimental dev-only package" right?
The StrictMode syntax mostly looks good, it's a great start. I think the only thing I'd like is that const padding = '8px'; local reusability.
Issues:
- The hashes are wrong!
- Could use validation of sorting (it's more important than it may seem)
- What about sorting? I'm not 100% certain how it works.
| }, | ||
| "peerDependencies": { | ||
| "@swc/core": ">=1.3.0", | ||
| "react": ">=16.8.0" |
There was a problem hiding this comment.
Not sure why we need a peer dep here actually, but ^18.2.0 to match Atlassian…unless Compiled has this peer dep elsewhere then just match it.
packages/swc-plugin/src/index.ts
Outdated
| export interface CompiledSwcPluginOptions { | ||
| /** | ||
| * Import sources to transform CSS-in-JS from | ||
| * @default ["@compiled/react"] |
There was a problem hiding this comment.
I thought I defaulted this to ["@compiled/react", "@atlaskit/css"] elsewhere, could you check and match?
| * Prefix for generated classes' hashes | ||
| * @default undefined | ||
| */ | ||
| classHashPrefix?: string; |
There was a problem hiding this comment.
I could see dropping this in here, shouldn't be used in the first place.
| * Whether to process xcss usages | ||
| * @default true | ||
| */ | ||
| processXcss?: boolean; |
There was a problem hiding this comment.
I could see dropping this in here, shouldn't be overridden in the first place.
| * Prefix for generated CSS class names | ||
| * @default "_" | ||
| */ | ||
| classNamePrefix?: string; |
There was a problem hiding this comment.
I could see dropping this in here, shouldn't be overridden in the first place.
| }); | ||
| `; | ||
| const out = await transformResultString(code); | ||
| expect(out).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
Is there a way for a friendlier message, eg. I saw the "…not supported in Strict Mode…" before?
|
|
||
| const theme = { colors: { primary: 'red' } }; | ||
| const styles = css({ | ||
| color: theme.colors.primary |
There was a problem hiding this comment.
This might actually require refactor in a few places, but sparingly, that's fine.
| const primaryColor = 'red'; | ||
| const styles = cssMap({ | ||
| primary: { | ||
| color: primaryColor |
There was a problem hiding this comment.
This will definitely break in a few places, this is a "supported value" by default, it'd be ideal if at a minimum, this was supportable, because it's the only option (barring token(…) babel plugin type effort) for value reuse—local reuse is fairly necessary.
| }); | ||
| }); | ||
|
|
||
| describe('Comparison with Babel Plugin Behavior', () => { |
There was a problem hiding this comment.
This test doesn't particularly do what it says ti? Eg. the hashes are wrong.
Be lovely if it could be 100% hooked up, then we could just have a huge css({ … }) edge-case call and compare it 1:1!
There was a problem hiding this comment.
Another dumb comment, what about sorting? Does <CC> and <CS> do this and we don't have to build it into the plugin (unless we're doing extraction into CSS files?)
See: https://github.com/atlassian-labs/compiled/blob/master/packages/css/src/plugins/sort-atomic-style-sheet.ts, but I don't recall where/how that's used in Babel.
What is this change?
parcel-transform-swcstep that will come in another PR.Why are we making this change?
How are we making this change?
Change will be rolled out in 3 parts.
PR checklist
Don't delete me!
I have...
website/When it's verified stable