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: document can be null, not just undefined #22695

Merged
merged 1 commit into from
Nov 24, 2021
Merged

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Nov 4, 2021

Summary

Jest sets document to null, not undefined (or delete which the test from #11677 does) when tearing down the env. https://github.com/facebook/jest/blob/95f49691d3472d8187640f1b703209b93c2bcecc/packages/jest-environment-jsdom/src/index.ts#L135-L136

How did you test this change?

At work we get

/workspace/node_modules/react-dom/cjs/react-dom.development.js:3905
      var evt = document.createEvent('Event');
                         ^
TypeError: Cannot read properties of null (reading 'createEvent')
    at Object.invokeGuardedCallbackDev (/workspace/node_modules/react-dom/cjs/react-dom.development.js:3905:26)
    at invokeGuardedCallback (/workspace/node_modules/react-dom/cjs/react-dom.development.js:4056:31)
    at flushPassiveEffectsImpl (/workspace/node_modules/react-dom/cjs/react-dom.development.js:23543:11)
    at unstable_runWithPriority (/workspace/node_modules/scheduler/cjs/scheduler.development.js:468:12)
    at runWithPriority$1 (/workspace/node_modules/react-dom/cjs/react-dom.development.js:11276:10)
    at flushPassiveEffects (/workspace/node_modules/react-dom/cjs/react-dom.development.js:23447:14)
    at Object.<anonymous>.flushWork (/workspace/node_modules/react-dom/cjs/react-dom-test-utils.development.js:992:10)
    at Immediate.<anonymous> (/workspace/node_modules/react-dom/cjs/react-dom-test-utils.development.js:1003:11)
    at processImmediate (node:internal/timers:464:21)
    at process.topLevelDomainCallback (node:domain:152:15)
    at process.callbackTrampoline (node:internal/async_hooks:128:24)

sometimes. I've been unable to track down where in our code we're missing a teardown (unfortunately the stack only points to react internals, no trace back to user code), but the error can be cleaner 🙂

@sizebot
Copy link

sizebot commented Nov 4, 2021

Comparing: 51c558a...addfc4c

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 129.55 kB 129.55 kB = 41.43 kB 41.43 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 134.52 kB 134.52 kB = 42.89 kB 42.89 kB
facebook-www/ReactDOM-prod.classic.js = 423.36 kB 423.36 kB = 78.05 kB 78.05 kB
facebook-www/ReactDOM-prod.modern.js = 411.92 kB 411.92 kB = 76.30 kB 76.30 kB
facebook-www/ReactDOMForked-prod.classic.js = 423.36 kB 423.36 kB = 78.06 kB 78.06 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against addfc4c

@gaearon gaearon merged commit 4e6eec6 into facebook:main Nov 24, 2021
@SimenB SimenB deleted the patch-2 branch November 24, 2021 21:03
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Dec 14, 2021
Summary:
This sync includes the following changes:
- **[f2a59df48](facebook/react@f2a59df48 )**: Remove unstableAvoidThisFallback from OSS ([#22884](facebook/react#22884)) //<salazarm>//
- **[24dd07bd2](facebook/react@24dd07bd2 )**: Add custom element property support behind a flag ([#22184](facebook/react#22184)) //<Joey Arhar>//
- **[72e48b8e1](facebook/react@72e48b8e1 )**: Fix: Don't skip writing updated package.json //<Andrew Clark>//
- **[e39b2c899](facebook/react@e39b2c899 )**: Fix peer deps for use-sync-external-store //<Andrew Clark>//
- **[ec78b135f](facebook/react@ec78b135f )**: Don't override use-sync-external-store peerDeps ([#22882](facebook/react#22882)) //<Andrew Clark>//
- **[5041c37d2](facebook/react@5041c37d2 )**: Remove hydrate option from createRoot ([#22878](facebook/react#22878)) //<salazarm>//
- **[3f9480f0f](facebook/react@3f9480f0f )**: enable continuous replay flag ([#22863](facebook/react#22863)) //<salazarm>//
- **[4729ff6d1](facebook/react@4729ff6d1 )**: Implement identifierPrefix option for useId ([#22855](facebook/react#22855)) //<Andrew Clark>//
- **[ed00d2c3d](facebook/react@ed00d2c3d )**: Remove unused flag ([#22854](facebook/react#22854)) //<Dan Abramov>//
- **[0cc724c77](facebook/react@0cc724c77 )**: update ReactFlightWebpackPlugin to be compatiable with webpack v5 ([#22739](facebook/react#22739)) //<Michelle Chen>//
- **[4e6eec69b](facebook/react@4e6eec69b )**: fix: document can be `null`, not just `undefined` ([#22695](facebook/react#22695)) //<Simen Bekkhus>//

Changelog:
[General][Changed] - React Native sync for revisions c1220eb...a049aa0

jest_e2e[run_all_tests]

Reviewed By: rickhanlonii

Differential Revision: D33062386

fbshipit-source-id: 37e497947efad5696c251096da8a92ccdc6dcea7
@gaearon gaearon mentioned this pull request Mar 29, 2022
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants