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

chore: add babel plugin to make sure Jest is unaffected by fake Promise implementations #7225

Merged
merged 13 commits into from
Oct 24, 2018

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Oct 20, 2018

Summary

Since users have access to the same globals that Jest does inside the sandbox, they can mess with things Jest needs to function, like doing global.Promise = undefined. Our approach of just assigning our own local Promise is brittle in that we have to remember to do it, and it breaks in babel 7. I've verified that at least for Babel 6, this plugin changes the injected asynToGenerator helper from Babel, so the approach should hopefully work for Babel 7 as well.

See #7016 (comment)
/cc @loganfsmyth

Test plan

I've looked at the generated code, and it looks correct. I've never written a babel plugin before though, so there might be gotchas?
🤞 for CI

@SimenB

This comment has been minimized.

@SimenB SimenB mentioned this pull request Oct 20, 2018
6 tasks
@SimenB

This comment has been minimized.

title: testResult.testPath[testResult.testPath.length - 1],
};
});
const assertionResults: Array<AssertionResult> = runResult.testResults.map(
Copy link
Member Author

Choose a reason for hiding this comment

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

ignore whitespace when looking at this diff. I only added the type annotation

@@ -9,7 +9,7 @@

export type DoneFn = (reason?: string | Error) => void;
export type BlockFn = () => void;
export type BlockName = string | Function;
export type BlockName = string;
Copy link
Member Author

Choose a reason for hiding this comment

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

@aaronabramov whenever you find the time, does this make sense? We do not support just a function in describe.

image

@SimenB

This comment has been minimized.

@SimenB
Copy link
Member Author

SimenB commented Oct 24, 2018

This has been sitting without objections for a few days, so I'll merge if CI is happy after the rebase

@SimenB SimenB merged commit 187ca9e into jestjs:master Oct 24, 2018
@SimenB SimenB deleted the babel-plugin-jest-promise branch October 24, 2018 14:02
@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 May 12, 2021
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.

4 participants