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: remove require.requireActual and require.requireMock #9854

Merged
merged 2 commits into from
May 2, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Apr 21, 2020

Summary

It's been deprecated for a few years now, let's clean it up.

Might land the change to _createJestObjectFor on master separately, as it cleans up #9849 for the ESM code path done

Test plan

Green CI

@SimenB SimenB force-pushed the remove-require-requireActual branch from e1ae49d to 7f3cd83 Compare April 22, 2020 08:08
Copy link
Member

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

👍

@arty-name
Copy link

Docs still mention it

@SimenB
Copy link
Member Author

SimenB commented May 22, 2020

@arty-name it has jest.requireActual which is not removed

@arty-name
Copy link

Wow, that's great news! Thanks!

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jul 16, 2020
A reversion of e40c020 and 62621ef. Unfortunately, Jest 26 (the
latest), which we'll take in an upcoming commit, doesn't support
`jest-expo` at 37 [1].

We can't go past 37 (e.g., to 38.0.2, the latest) until we upgrade
to React Native v0.62 (that's zulip#3782). That's because jest-expo's
"react" peer dependency was bumped from ~16.9.0 to ~16.11.0 in
expo/expo#8310 [2], and we must do that React upgrade atomically
with our RN upgrade. The "react-native" peer dependency wasn't
touched; it remained at "*". So, I'm left unsure whether it was
intentional to drop support for RN v0.61 [3].

Ah well. As we were aware in e40c020:

"""
If `jest-expo` turns out to be buggy, or the dependency requirements
get even more tangled or burdensome, we should feel free to abandon
this effort; it's not terrible to have to add boring mocks.
"""

We may consider adding it back in as a followup to zulip#3782.

Run our usual `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

[1] We get errors about jest-expo using `require.requireActual`,
    which was removed in jestjs/jest#9854, out in v26.0.0-alpha.0.
[2] expo/expo@a4cabf30a#diff-4a85ebd1069aff25ee2e5f2b004281ccR33
[3] See react-native-webview/react-native-webview#1445.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Aug 12, 2020
A reversion of e40c020 and 62621ef. Unfortunately, Jest 26 (the
latest), which we'll take in an upcoming commit, doesn't support
`jest-expo` at 37 [1].

We can't go past 37 (e.g., to 38.0.2, the latest) until we upgrade
to React Native v0.62 (that's zulip#3782). That's because jest-expo's
"react" peer dependency was bumped from ~16.9.0 to ~16.11.0 in
expo/expo#8310 [2], and we must do that React upgrade atomically
with our RN upgrade. The "react-native" peer dependency wasn't
touched; it remained at "*". So, I'm left unsure whether it was
intentional to drop support for RN v0.61 [3].

Ah, well. As I said in e40c020:

"""
If `jest-expo` turns out to be buggy, or the dependency requirements
get even more tangled or burdensome, we should feel free to abandon
this effort; it's not terrible to have to add boring mocks.
"""

We may consider adding it back in as a followup to zulip#3782.

Run our usual `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

[1] We get errors about jest-expo using `require.requireActual`,
    which was removed in jestjs/jest#9854, out in v26.0.0-alpha.0.
[2] expo/expo@a4cabf30a#diff-4a85ebd1069aff25ee2e5f2b004281ccR33
[3] See react-native-webview/react-native-webview#1445.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Aug 14, 2020
A reversion of e40c020 and 62621ef. Unfortunately, Jest 26 (the
latest), which we'll take in an upcoming commit, doesn't support
`jest-expo` at 37 [1].

We can't go past 37 (e.g., to 38.0.2, the latest) until we upgrade
to React Native v0.62 (that's zulip#3782). That's because jest-expo's
"react" peer dependency was bumped from ~16.9.0 to ~16.11.0 in
expo/expo#8310 [2], and we must do that React upgrade atomically
with our RN upgrade. The "react-native" peer dependency wasn't
touched; it remained at "*". So, I'm left unsure whether it was
intentional to drop support for RN v0.61 [3].

Ah, well. As I said in e40c020:

"""
If `jest-expo` turns out to be buggy, or the dependency requirements
get even more tangled or burdensome, we should feel free to abandon
this effort; it's not terrible to have to add boring mocks.
"""

We may consider adding it back in as a followup to zulip#3782.

Run our usual `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

[1] We get errors about jest-expo using `require.requireActual`,
    which was removed in jestjs/jest#9854, out in v26.0.0-alpha.0.
[2] expo/expo@a4cabf30a#diff-4a85ebd1069aff25ee2e5f2b004281ccR33
[3] See react-native-webview/react-native-webview#1445.
@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 11, 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.

5 participants