feat: add support for makeResetStyles transforms #236
Conversation
📊 Bundle size report🤖 This report was generated against 48239dc8719e4ac723d517291e5d9a1f866e42c6 |
layershifter
force-pushed
the
feat/make-reset-styles
branch
2 times, most recently
from
5fa5ade to
d02e321
Compare
layershifter
force-pushed
the
feat/make-reset-styles
branch
4 times, most recently
from
a132e3a to
791a7e1
Compare
| "jestConfig": "packages/webpack-loader/jest.config.js", | ||
| "passWithNoTests": true | ||
| }, | ||
| "dependsOn": [{ "target": "build", "projects": "dependencies" }] |
There was a problem hiding this comment.
I changed @griffel/react to use externals, so we need to build less code during each test and we don't need to build packages before
| it('makeStyles', () => { | ||
| expect(shouldTransformSourceCode(`import { makeStyles } from "@griffel/react"`, undefined)).toBe(true); | ||
| expect(shouldTransformSourceCode(`import { Button } from "@fluentui/react"`, undefined)).toBe(false); | ||
| }); |
There was a problem hiding this comment.
The same test, just moved into describe block
| it('makeStyles', () => { | ||
| expect( | ||
| shouldTransformSourceCode(`import { makeStyles } from "@griffel/react"`, [ | ||
| { moduleSource: '@griffel/react', importName: 'makeStyles' }, | ||
| ]), | ||
| ).toBe(true); | ||
| expect( | ||
| shouldTransformSourceCode(`import { createStyles } from "make-styles"`, [ | ||
| { moduleSource: 'make-styles', importName: 'createStyles' }, | ||
| ]), | ||
| ).toBe(true); | ||
|
|
||
| expect( | ||
| shouldTransformSourceCode(`import { Button } from "@fluentui/react"`, [ | ||
| { moduleSource: '@griffel/react', importName: 'makeStyles' }, | ||
| ]), | ||
| ).toBe(false); | ||
| }); |
There was a problem hiding this comment.
The same test, just moved into describe block
| enum FunctionKinds { | ||
| makeStyles = 'makeStyles', | ||
| makeResetStyles = 'makeResetStyles', | ||
| } |
There was a problem hiding this comment.
Leaves the space for transforms of makeStaticStyles
layershifter
force-pushed
the
feat/make-reset-styles
branch
2 times, most recently
from
ba2d10a to
5aa1472
Compare
layershifter
force-pushed
the
feat/make-reset-styles
branch
from
5aa1472 to
755cccf
Compare
| throw definitionsPath.buildCodeFrameError('makeStyles() function accepts only an object as a param'); | ||
| if (functionKind === 'makeStyles') { | ||
| if (!definitionsPath.isObjectExpression()) { | ||
| throw definitionsPath.buildCodeFrameError('makeStyles() function accepts only an object as a param'); |
There was a problem hiding this comment.
both makeStyles and makeResetStyles accept an object only as param right ?
There was a problem hiding this comment.
Correct, will check it.
There was a problem hiding this comment.
Restored to the previous condition, don't remember why it looks like that
| importName: string; | ||
| resetImportName?: string; |
There was a problem hiding this comment.
| importName: string; | |
| resetImportName?: string; | |
| makeStylesimportName: string; | |
| makeResetStylesImportName?: string; |
it's a bit longer, but it might make things clearer in the plugin code
There was a problem hiding this comment.
It's the part of public API so a change there will be BC, that's why it's like that 🥲
| // Fallback to "makeStyles" if options were not provided | ||
| const imports = modules ? modules.map(module => module.importName).join('|') : 'makeStyles'; | ||
| const imports = modules | ||
| ? modules.flatMap(module => [module.importName, module.resetImportName || 'makeResetStyles']).join('|') |
There was a problem hiding this comment.
why does module.resetImportName need to default to makeResetStyles when importName doesn't need to default to makeStyles ?
There was a problem hiding this comment.
importNameis a required paramresetImportNameis optional to avoid BC 😕
ling1726
left a comment
ling1726
left a comment
There was a problem hiding this comment.
Changes look more or less OK to me, it's simply just extending the plugin state to differentiate between the two functions and then handling the replacement appropriately.
Evaluation hasn't changed at all from what I can see
Yup, that's correct. |
layershifter
force-pushed
the
feat/make-reset-styles
branch
from
bc0f31f to
d04b624
Compare
2 participants
Fixes #227.
This PR adds new tests and modifies
@griffel/babel-presetor@griffel/webpack-loaderto handlemakeResetStylestransforms.resetImportNameadded tomodulesoptions of Babel preset and will be documented in a separate PR.