-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: Fix user override of customExportConditions in custom env subclasses #13989
Conversation
Would be useful to add configuration example you provided to the |
@mrazauskas Believe this is present, but happy to improve. Note: |
I took one more look and opened a PR to adding the examples. See #13991 |
@huntie is it? 😀 |
So it turns out that neither of these patterns allows overriding via ...because the property initialiser
|
499117d
to
738e982
Compare
738e982
to
a42ae3d
Compare
PR is updated and now makes a small functional fix along with test updates (see previous comment), preferring a non-breaking approach. @mrazauskas @SimenB Ready for review. |
a42ae3d
to
67be705
Compare
Ready for review. |
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.
👍
@huntie is this something you need released? |
@SimenB Would be nice, but it's not super urgent — especially as Package Exports is shipping experimentally at first. Not a direct dependency for the config updates in React Native, just the edge case will get fixed under the next version :). |
Summary: Updates the default set of `"exports"` condition names in our `ReactNativeEnv` for Jest, so that it aligns with the defaults in React Native CLI (react-native-community/cli#1862). Also includes a subtle update to how this is accomplished. Instead of overriding `exportConditions()`, we assign to the underlying class property — this allows users (once jestjs/jest#13989 is merged) to override `customExportConditions` via [`testEnvironmentOptions`](https://jestjs.io/docs/configuration#testenvironmentoptions-object). ```js preset: 'react-native', testEnvironmentOptions: { customExportConditions: ['test', 'react-native'], }, ``` Changelog: [Internal] Reviewed By: jacdebug Differential Revision: D43879056 fbshipit-source-id: 86fffe2b5fdf9d8492d25b8b12a78be75b5fa3be
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. |
Summary
Adjusts behaviour in
jest-environment-jsdom
andjest-environment-node
(no breaking changes).With
customExportConditions
exposed as a public property on each env class, and as previously tested indeno.test.mjs
, subclasses can override"exports"
conditions in two ways:Previously, both overriding the property or reimplementing
exportConditions()
would mean that when a user attempted to override this in Jest config, this was ignored.These changes make sure this behaviour works for the property assignment case — and is covered by tests. We plan to make this load-bearing in the React Native Jest preset for 0.72.
Changes:
jest-environment-jsdom
,jest-environment-node
to use thecustomExportConditions
property as a default, which will be overridden if set inconfig.projectConfig.testEnvironmentOptions
.deno.test.mjs
/deno-env.js
tocustom-env
(more generic).custom-env-override-conditions.test.mjs
with new test case (tests via@jest-environment-options
doc comment).Test plan
✅ Updated tests pass