-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Storyshots: Fix mdx transform #10223
Conversation
after we transform from mdx to js... we need to name the file extension js so our other jest transforms transform the js correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thank you for the fix! 🙏 Please see my comment inline.
Co-Authored-By: Michael Shilman <[email protected]>
@rileylnapier Hmm this seems to break CI. @Hypnosphi can you take a quick look? |
@rileylnapier can you please describe the original issue you're solving here? Right now the JS transforms are applied to mdx, it's done in |
The issue that I had is that after my mdx gets transformed into JS. The JS that results has things like With this being said... I think my team and I are probably just going to to CSF as the MDX stuff is too tricky with the whitespace errors and no typescript typechecking. Def love the idea, but I don't think its what my team will be into. i'm also using ts-jest btw, not babel-jest if that would make a differnce |
@rileylnapier can you please share your jest config? Or, even better, create a reproduction repo? We have JS transforms in our config, and they're definitely applied |
this was my config:
|
@rileylnapier can you please also copy a few lines above your screenshots? Looks like they're saying something about ts-jest |
ts-jest[jest-transformer] (WARN) Got a unknown file type to compile (file: /Users/rileynapier/trycourier/frontend/packages/studio I guess this is why i needed the file to be .js so tsjest would transpile it after? |
Co-Authored-By: Filipp Riabchun <[email protected]>
OK I got it, your solution should work better indeed. Actually, we can replace |
addons/docs/jest-transform-mdx.js
Outdated
|
||
const jsFileName = `${filename}.js`; | ||
|
||
return getNextTransformer(filename, config).transformSource(jsFileName, result, instrument); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return getNextTransformer(filename, config).transformSource(jsFileName, result, instrument); | |
return new ScriptTransformer(config).transformSource(jsFileName, result, instrument); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we would want to delete getNextTransformer
then right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
so to be clear. this was not an issue when using babel-jest right? because babel doesn't have file extension restrictions right? so you tell babel to transpile a file with .mdx and say its js, babel is cool with that. however, something like ts-jest does care about the file extension and giving it a .mdx file it gets mad. i did have to enable |
Storyshots: Fix mdx transform
after we transform from mdx to js... we need to name the file extension js so our other jest transforms transform the js correctly.
Issue:
What I did
How to test
If your answer is yes to any of these, please make sure to include it in your PR.