Skip to content

Make host context use null as empty and only error in dev #25609

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

Merged
merged 1 commit into from
Nov 1, 2022

Conversation

sebmarkbage
Copy link
Collaborator

Makes it slightly more blazing.

No host config actually uses null as any useful and we use this as a placeholder value anyway. It's also better for perf since it doesn't let two different hidden classes pass around. It's also a magic place holder that triggers error if we do try to access anything from it.

return (c: any);
}

function getCurrentRootHostContainer(): null | Container {
const container = rootInstanceStackCursor.current;
return container === NO_CONTEXT ? null : ((container: any): Container);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one is precious.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was busy!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I mean it's fair, once a pattern is established it tends to spread.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

Blazing means good

Copy link
Collaborator

@gnoff gnoff left a comment

Choose a reason for hiding this comment

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

we could always use more blazing

@sebmarkbage
Copy link
Collaborator Author

It's 700x faster.

@sizebot
Copy link

sizebot commented Nov 1, 2022

Comparing: 5f7ef8c...2731b4c

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 = 153.80 kB 153.65 kB = 48.94 kB 48.90 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 155.72 kB 155.57 kB = 49.57 kB 49.51 kB
facebook-www/ReactDOM-prod.classic.js = 531.17 kB 530.61 kB = 94.76 kB 94.67 kB
facebook-www/ReactDOM-prod.modern.js = 516.42 kB 515.87 kB = 92.61 kB 92.50 kB
facebook-www/ReactDOMForked-prod.classic.js = 531.17 kB 530.61 kB = 94.76 kB 94.68 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-test-renderer/cjs/react-test-renderer.production.min.js = 99.84 kB 99.61 kB = 30.74 kB 30.67 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.production.min.js = 99.78 kB 99.55 kB = 30.71 kB 30.63 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.production.min.js = 99.75 kB 99.53 kB = 30.71 kB 30.63 kB
oss-experimental/react-test-renderer/umd/react-test-renderer.production.min.js = 100.07 kB 99.85 kB = 31.14 kB 31.09 kB
oss-stable/react-test-renderer/umd/react-test-renderer.production.min.js = 100.01 kB 99.79 kB = 31.10 kB 31.06 kB
oss-stable-semver/react-test-renderer/umd/react-test-renderer.production.min.js = 99.99 kB 99.77 kB = 31.10 kB 31.06 kB

Generated by 🚫 dangerJS against 2731b4c

@sebmarkbage sebmarkbage merged commit 8e69bc4 into facebook:main Nov 1, 2022
GrinZero added a commit to GrinZero/react that referenced this pull request Nov 7, 2022
* 'main' of ssh://github.com/GrinZero/react: (163 commits)
  Do not unmount layout effects if ancestor Offscreen is hidden (facebook#25628)
  Remove check in renderDidSuspendDelayIfPossible (facebook#25630)
  [ServerRenderer] Move fizz external runtime implementation to react-dom-bindings (facebook#25617)
  Unwrap sync resolved thenables without suspending  (facebook#25615)
  refactor isHostResourceType to not receive the context from reconciler and not leak types (facebook#25610)
  Make host context use null as empty and only error in dev (facebook#25609)
  [Float] handle resource Resource creation inside svg context (facebook#25599)
  Allow uncached IO to stablize (facebook#25561)
  [ServerRenderer] Setup for adding data attributes streaming format (facebook#25567)
  Do not unmount layout effects on initial Offscreen mount (facebook#25592)
  Fix type check for null (facebook#25595)
  Clean up vestige of useOpaqueIdentifier (facebook#25587)
  Extract logic for detecting bad fallback to helper
  Split suspended work loop logic into separate functions
  In work loop, add enum of reasons for suspending
  Strict Mode: Reuse memoized result from first pass (facebook#25583)
  Detect and warn if use(promise) is wrapped with try/catch block (facebook#25543)
  Remove old react-fetch, react-fs and react-pg libraries (facebook#25577)
  Try assigning fetch to globalThis if global assignment fails (facebook#25571)
  [Float] handle noscript context for Resources (facebook#25559)
  ...

# Conflicts:
#	scripts/rollup/build.js
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 7, 2022
Summary:
This sync includes the following changes:
- **[4bd245e9e](facebook/react@4bd245e9e )**: Do not unmount layout effects if ancestor Offscreen is hidden ([#25628](facebook/react#25628)) //<Samuel Susla>//
- **[df61e708c](facebook/react@df61e708c )**: Remove check in renderDidSuspendDelayIfPossible ([#25630](facebook/react#25630)) //<Andrew Clark>//
- **[1a08f1478](facebook/react@1a08f1478 )**: [ServerRenderer] Move fizz external runtime implementation to react-dom-bindings ([#25617](facebook/react#25617)) //<mofeiZ>//
- **[1a902623a](facebook/react@1a902623a )**: Unwrap sync resolved thenables without suspending  ([#25615](facebook/react#25615)) //<Andrew Clark>//
- **[4ea063b56](facebook/react@4ea063b56 )**: refactor isHostResourceType to not receive the context from reconciler and not leak types ([#25610](facebook/react#25610)) //<Josh Story>//
- **[8e69bc45a](facebook/react@8e69bc45a )**: Make host context use null as empty and only error in dev ([#25609](facebook/react#25609)) //<Sebastian Markbåge>//
- **[5f7ef8c4c](facebook/react@5f7ef8c4c )**: [Float] handle resource Resource creation inside svg context ([#25599](facebook/react#25599)) //<Josh Story>//
- **[36426e6cb](facebook/react@36426e6cb )**: Allow uncached IO to stablize ([#25561](facebook/react#25561)) //<Andrew Clark>//
- **[6883d7944](facebook/react@6883d7944 )**: [ServerRenderer] Setup for adding data attributes streaming format ([#25567](facebook/react#25567)) //<mofeiZ>//

Changelog:
[General][Changed] - React Native sync for revisions ab075a2...4bd245e

jest_e2e[run_all_tests]

Reviewed By: GijsWeterings

Differential Revision: D41028209

fbshipit-source-id: a67fdcd441ddd50784f7c1ce402eaecdb5e3126d
rickhanlonii pushed a commit that referenced this pull request Dec 3, 2022
Makes it slightly more blazing.

No host config actually uses null as any useful and we use this as a
placeholder value anyway. It's also better for perf since it doesn't let
two different hidden classes pass around. It's also a magic place holder
that triggers error if we do try to access anything from it.
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This sync includes the following changes:
- **[4bd245e9e](facebook/react@4bd245e9e )**: Do not unmount layout effects if ancestor Offscreen is hidden ([facebook#25628](facebook/react#25628)) //<Samuel Susla>//
- **[df61e708c](facebook/react@df61e708c )**: Remove check in renderDidSuspendDelayIfPossible ([facebook#25630](facebook/react#25630)) //<Andrew Clark>//
- **[1a08f1478](facebook/react@1a08f1478 )**: [ServerRenderer] Move fizz external runtime implementation to react-dom-bindings ([facebook#25617](facebook/react#25617)) //<mofeiZ>//
- **[1a902623a](facebook/react@1a902623a )**: Unwrap sync resolved thenables without suspending  ([facebook#25615](facebook/react#25615)) //<Andrew Clark>//
- **[4ea063b56](facebook/react@4ea063b56 )**: refactor isHostResourceType to not receive the context from reconciler and not leak types ([facebook#25610](facebook/react#25610)) //<Josh Story>//
- **[8e69bc45a](facebook/react@8e69bc45a )**: Make host context use null as empty and only error in dev ([facebook#25609](facebook/react#25609)) //<Sebastian Markbåge>//
- **[5f7ef8c4c](facebook/react@5f7ef8c4c )**: [Float] handle resource Resource creation inside svg context ([facebook#25599](facebook/react#25599)) //<Josh Story>//
- **[36426e6cb](facebook/react@36426e6cb )**: Allow uncached IO to stablize ([facebook#25561](facebook/react#25561)) //<Andrew Clark>//
- **[6883d7944](facebook/react@6883d7944 )**: [ServerRenderer] Setup for adding data attributes streaming format ([facebook#25567](facebook/react#25567)) //<mofeiZ>//

Changelog:
[General][Changed] - React Native sync for revisions ab075a2...4bd245e

jest_e2e[run_all_tests]

Reviewed By: GijsWeterings

Differential Revision: D41028209

fbshipit-source-id: a67fdcd441ddd50784f7c1ce402eaecdb5e3126d
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants