-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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