-
Notifications
You must be signed in to change notification settings - Fork 46.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Fiber][Float] Error when a host fiber changes "flavor"
Host Components can exist as four semantic types 1. regular Components (Vanilla) 2. singleton Components 2. hoistable components 3. resources Each of these component types have their own rules related to mounting and reconciliation however they are not direclty modeled as their own unique fiber type. This is partly for code size but also because reconciling the inner type of these components would be in a very hot path in fiber creation and reconciliation and it's just not practical to do this logic check here. Right now we have three Fiber types used to implement these 4 concepts but we probably need to reconsider the model and think of Host Components as a single fiber type with an inner implementation. Once we do this we can regularize things like transitioning between a resource and a regular component or a singleton and a hoistable instance. The cases where these transitions happen today aren't particularly common but they can be observed and currently the handling of these transitions is incmplete at best and buggy at worst. The most egregious case is the link type. This can be a regular component (stylesheet without precedence) a hositable component (non stylesheet link tags) or a resource (stylesheet with a precedence) and if you have a single jsx slot that tries to reconcile transitions between these types it just doesn't work well. This commit adds an error for when a Hoistable goes from Instance to Resource. This is the most urgent because it is the easiest to hit but doesn't add much overhead in hot paths detecting type shifting to and from regular components is harder to do efficiently because we don't want to reevaluate the type on every update for host components which is currently not required and would add overhead to a very hot path singletons can't really type shift in their one practical implementation (DOM) so they are only a problem in theroy not practice
- Loading branch information
Showing
5 changed files
with
253 additions
and
35 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
123 changes: 123 additions & 0 deletions
123
packages/react-dom/src/__tests__/ReactDOMHostComponentTransitions-test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| /** | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| * @emails react-core | ||
| * @jest-environment ./scripts/jest/ReactDOMServerIntegrationEnvironment | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| let JSDOM; | ||
| let React; | ||
| let ReactDOMClient; | ||
| let container; | ||
| let waitForAll; | ||
|
|
||
| describe('ReactDOM HostSingleton', () => { | ||
| beforeEach(() => { | ||
| jest.resetModules(); | ||
| JSDOM = require('jsdom').JSDOM; | ||
| // Test Environment | ||
| const jsdom = new JSDOM( | ||
| '<!DOCTYPE html><html><head></head><body><div id="container">', | ||
| { | ||
| runScripts: 'dangerously', | ||
| }, | ||
| ); | ||
| global.window = jsdom.window; | ||
| global.document = jsdom.window.document; | ||
| container = global.document.getElementById('container'); | ||
|
|
||
| React = require('react'); | ||
| ReactDOMClient = require('react-dom/client'); | ||
|
|
||
| const InternalTestUtils = require('internal-test-utils'); | ||
| waitForAll = InternalTestUtils.waitForAll; | ||
| }); | ||
|
|
||
| it('errors when a hoistable component becomes a Resource', async () => { | ||
| const errors = []; | ||
| function onError(e) { | ||
| errors.push(e.message); | ||
| } | ||
| const root = ReactDOMClient.createRoot(container, { | ||
| onUncaughtError: onError, | ||
| }); | ||
|
|
||
| root.render( | ||
| <div> | ||
| <link rel="preload" href="bar" as="style" /> | ||
| </div>, | ||
| ); | ||
| await waitForAll([]); | ||
|
|
||
| root.render( | ||
| <div> | ||
| <link rel="stylesheet" href="bar" precedence="default" /> | ||
| </div>, | ||
| ); | ||
| await waitForAll([]); | ||
| if (__DEV__) { | ||
| expect(errors).toEqual([ | ||
| `Expected <link> not to update to be updated to a stylehsheet with precedence. Check the \`rel\`, \`href\`, and \`precedence\` props of this component. Alternatively, check whether two different <link> components render in the same slot or share the same key. | ||
| - <link rel=\"preload\" href=\"bar\" ... /> | ||
| + <link rel=\"stylesheet\" href=\"bar\" precedence=\"default\" />`, | ||
| ]); | ||
| } else { | ||
| expect(errors).toEqual([ | ||
| 'Expected <link> not to update to be updated to a stylehsheet with precedence. Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different <link> components render in the same slot or share the same key.', | ||
| ]); | ||
| } | ||
| }); | ||
|
|
||
| it('errors when a hoistable Resource becomes an instance', async () => { | ||
| const errors = []; | ||
| function onError(e) { | ||
| errors.push(e.message); | ||
| } | ||
| const root = ReactDOMClient.createRoot(container, { | ||
| onUncaughtError: onError, | ||
| }); | ||
|
|
||
| root.render( | ||
| <div> | ||
| <link rel="stylesheet" href="bar" precedence="default" /> | ||
| </div>, | ||
| ); | ||
| await waitForAll([]); | ||
| const event = new window.Event('load'); | ||
| const preloads = document.querySelectorAll('link[rel="preload"]'); | ||
| for (let i = 0; i < preloads.length; i++) { | ||
| const node = preloads[i]; | ||
| node.dispatchEvent(event); | ||
| } | ||
| const stylesheets = document.querySelectorAll('link[rel="preload"]'); | ||
| for (let i = 0; i < stylesheets.length; i++) { | ||
| const node = stylesheets[i]; | ||
| node.dispatchEvent(event); | ||
| } | ||
|
|
||
| root.render( | ||
| <div> | ||
| <link rel="foo" href="bar" /> | ||
| </div>, | ||
| ); | ||
| await waitForAll([]); | ||
| if (__DEV__) { | ||
| expect(errors).toEqual([ | ||
| `Expected stylesheet with precedence to not be updated to a different kind of <link>. Check the \`rel\`, \`href\`, and \`precedence\` props of this component. Alternatively, check whether two different <link> components render in the same slot or share the same key. | ||
| - <link rel=\"stylesheet\" href=\"bar\" precedence=\"default\" /> | ||
| + <link rel=\"foo\" href=\"bar\" />`, | ||
| ]); | ||
| } else { | ||
| expect(errors).toEqual([ | ||
| 'Expected stylesheet with precedence to not be updated to a different kind of <link>. Check the `rel`, `href`, and `precedence` props of this component. Alternatively, check whether two different <link> components render in the same slot or share the same key.', | ||
| ]); | ||
| } | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters