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.

Sign up for GitHub

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

Jump to bottom

refactor isHostResourceType to not receive the context from reconciler and not leak types #25610

Merged
merged 7 commits into from
Nov 2, 2022
Merged

refactor isHostResourceType to not receive the context from reconciler and not leak types #25610

merged 7 commits into from
Nov 2, 2022

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Nov 1, 2022 •
edited
Loading

type validateDOMNesting
move isHostResourceType to ReactDOMHostConfig
type AncestorInfo
refactor resourceFormOnly into ancestorInfo.containerTagInScope
provide hostContext from reconciler

@sizebot
Copy link

sizebot commented Nov 1, 2022 •
edited
Loading

Comparing: 5f7ef8c...46d7b7f

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.80 kB +0.02% 48.94 kB 48.95 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 155.72 kB 155.72 kB = 49.57 kB 49.57 kB
facebook-www/ReactDOM-prod.classic.js = 531.17 kB 531.21 kB +0.01% 94.76 kB 94.78 kB
facebook-www/ReactDOM-prod.modern.js = 516.42 kB 516.47 kB +0.02% 92.61 kB 92.63 kB
facebook-www/ReactDOMForked-prod.classic.js = 531.17 kB 531.21 kB +0.01% 94.76 kB 94.78 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 46d7b7f

sebmarkbage
sebmarkbage approved these changes Nov 1, 2022
View reviewed changes
Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

nobody:

sizebot: +0.01%

sebmarkbage
sebmarkbage reviewed Nov 1, 2022
View reviewed changes
packages/react-dom-bindings/src/client/validateDOMNesting.js Outdated
oldInfo: ?AncestorInfoDevType,
tag: string,
) {
const AncestorInfoDev = {...(oldInfo || emptyAncestorInfoDev)};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this capitalized. Seems like an odd naming convention?

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 thought so too. I think it's meant to function like a singleton kind of like how Dispatcher is capitalized. It made my type name worse though so I changed it but then changed it back...

It's not a singleton but in memory layout but how it is accessed it's sort of a single global value so it made sense in that (possibly wildly inaccurate) mental framing and I restored it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

waitaminute....

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 did a find replace and capitalized it all myself and then convinced myself it was always that way. brb...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤦

@gnoff gnoff merged commit 4ea063b into facebook:main Nov 2, 2022
@gnoff gnoff deleted the refactor-hostcontext-read branch November 2, 2022 00:14
GrinZero added a commit to GrinZero/react that referenced this pull request Nov 7, 2022
@GrinZero
Merge branch 'main' of ssh://github.com/GrinZero/react
6049258 
* '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
@sammy-SC @facebook-github-bot
React Native sync for revisions ab075a2...4bd245e
7884445 
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
@kassens kassens mentioned this pull request Nov 17, 2022
Closed
rickhanlonii pushed a commit that referenced this pull request Dec 3, 2022
@gnoff @rickhanlonii
refactor isHostResourceType to not receive the context from reconcile…
34958f7 
…r and not leak types (#25610)

type validateDOMNesting
move `isHostResourceType` to ReactDOMHostConfig
type `AncestorInfo`
refactor `resourceFormOnly` into `ancestorInfo.containerTagInScope`
provide hostContext from reconciler
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
@sammy-SC @OlimpiaZurek
React Native sync for revisions ab075a2...4bd245e
c71f69f 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebmarkbage sebmarkbage sebmarkbage approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gnoff @sizebot @sebmarkbage @facebook-github-bot