Skip to content
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

fix: Allow updating inline snapshots when test includes JSX #12760

Merged
merged 9 commits into from
Aug 19, 2022

Conversation

eps1lon
Copy link
Contributor

@eps1lon eps1lon commented Apr 27, 2022

Summary

Closes #11561

Note that the fix by @SimenB does not fix the added test because the test passes the babel config directly to babel-jest instead of being declared in a babel.config.js.

It seems to me that saveSnapshotsForFile should ultimately ensure it transforms the source with the same transformer that was passed to Jest. It looks like it's currently assuming Babel with pre-defined plugins.

Though we can't actually accept any transformer since their AST format may differ and we currently use the AST to find toMatchInlineSnapshot calls.

I guess for now we focus on fixing this when using Babel and keep the old behavior when Babel isn't used?

Test plan

  • yarn jest toMatchInlineSnapshotWithJSX
  • CI

@eps1lon eps1lon force-pushed the fix/update-inline-snapshot-jsx branch from 4fd47bd to 586ac6e Compare April 27, 2022 19:56
@eps1lon eps1lon force-pushed the fix/update-inline-snapshot-jsx branch from 586ac6e to 23ce2ca Compare April 27, 2022 20:11
@wmertens
Copy link
Contributor

I take it there's no workaround possible in the meantime?

@SimenB SimenB force-pushed the fix/update-inline-snapshot-jsx branch from 37ec078 to e8f607f Compare August 19, 2022 12:00
@SimenB SimenB marked this pull request as ready for review August 19, 2022 13:20
Comment on lines +109 to +122
if (error.message.includes('@babel/plugin-syntax-jsx')) {
try {
const jsxSyntaxPlugin: PluginItem = [
require.resolve('@babel/plugin-syntax-jsx'),
{},
// unique name to make sure Babel does not complain about a possible duplicate plugin.
'JSX syntax plugin added by Jest snapshot',
];
ast = parseSync(sourceFile, {
filename: sourceFilePath,
plugins: [...plugins, jsxSyntaxPlugin],
presets,
root: rootDir,
});
Copy link
Member

@SimenB SimenB Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eps1lon went for this (plus #13150)

Comment on lines +113 to +115
{},
// unique name to make sure Babel does not complain about a possible duplicate plugin.
'JSX syntax plugin added by Jest snapshot',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably don't need this as the error is due to this plugin missing

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally 🙂

@SimenB SimenB merged commit 6a90a2c into jestjs:main Aug 19, 2022
@eps1lon eps1lon deleted the fix/update-inline-snapshot-jsx branch August 19, 2022 14:32
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline Snapshots fail to update if test uses JSX
4 participants