[core] Fix useTheme import change #134
Conversation
* Ignore transforming useTheme reference * Fix the path to generate theme source
| import { expect } from 'chai'; | ||
| import createExtendSxProp from '../../src/createExtendSxProp'; | ||
|
|
||
| describe('createExtendSxProp', () => { | ||
| it('return the new copy of input', () => { | ||
| const original = { color: 'red' }; | ||
| expect(createExtendSxProp()(original)).to.not.equal(original); | ||
| expect(createExtendSxProp()(original)).to.equal(original); |
There was a problem hiding this comment.
@siriwatknp Hopefully this is correct.
There was a problem hiding this comment.
expect(createExtendSxProp()(original)).to.not.equal(original); is already correct (at least, it's my intention) to ensure that the result creates a new object the same as what extendSxProp does.
There was a problem hiding this comment.
I changed it because it was failing.
function extendSxProp(props) {
return props;
}It's returning the same object. So the reference won't be different.
There was a problem hiding this comment.
Opps, then it should be the implementation that should be fixed.
function extendSxProp(props) {
return { ...props };
}
| import { useTheme } from '../zero-styled'; | ||
| import { useTheme } from '@pigment-css/react'; | ||
|
|
||
| console.log(useTheme); |
There was a problem hiding this comment.
| console.log(useTheme); |
There was a problem hiding this comment.
Is the console.log intentional?
There was a problem hiding this comment.
This is part of the test to make sure we don't have the same issue that this PR fixes.
There was a problem hiding this comment.
I see, got it. Can you add a comment that it's intentional? I am sure others will ask if they see console.log
| @@ -347,7 +347,10 @@ export const plugin = createUnplugin<PigmentOptions, true>((options) => { | |||
| if (id.endsWith('styles.css')) { | |||
| return theme ? generateTokenCss(theme) : _code; | |||
| } | |||
| if (id.includes('pigment-css-react/theme')) { | |||
| if ( | |||
| id.includes('pigment-css-react/theme') || | |||
There was a problem hiding this comment.
I wonder what's pigment-css-react/theme is for?
There was a problem hiding this comment.
At some point, it was for local testing in workspaces. We can maybe clean this up separately.
| @@ -1,11 +1,10 @@ | |||
| import { describe } from 'yargs'; | |||
| expect(createExtendSxProp()(original)).to.not.equal(original); | ||
| expect(createExtendSxProp()(original)).not.to.equal(original); |
There was a problem hiding this comment.
Nice, fixed in the whole codebase: mui/material-ui@6fe271e, and in MUI X.
Fixes #124