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

Add enableSuspenseServerRenderer feature flag #13573

Merged
merged 1 commit into from
Sep 5, 2018
Merged

Add enableSuspenseServerRenderer feature flag #13573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 59 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerPlaceholders-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* 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
*/

'use strict';

const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');

let React;
let ReactDOM;
let ReactDOMServer;
let ReactFeatureFlags;

function initModules() {
// Reset warning cache.
jest.resetModuleRegistry();

ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.enableSuspense = true;
ReactFeatureFlags.enableSuspenseServerRenderer = true;

React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');

// Make them available to the helpers.
return {
ReactDOM,
ReactDOMServer,
};
}

const {resetModules, serverRender} = ReactDOMServerIntegrationUtils(
initModules,
);

describe('ReactDOMServerPlaceholders', () => {
beforeEach(() => {
resetModules();
});

it('should always render the fallback when a placeholder is encountered', async () => {
const Suspended = props => {
throw new Promise(() => {});
};
const e = await serverRender(
<React.Placeholder fallback={<div />}>
<Suspended />
</React.Placeholder>,
);

expect(e.tagName).toBe('DIV');
});
});
28 changes: 27 additions & 1 deletion packages/react-dom/src/server/ReactPartialRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,17 @@ import warningWithoutStack from 'shared/warningWithoutStack';
import checkPropTypes from 'prop-types/checkPropTypes';
import describeComponentFrame from 'shared/describeComponentFrame';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {warnAboutDeprecatedLifecycles} from 'shared/ReactFeatureFlags';
import {
warnAboutDeprecatedLifecycles,
enableSuspenseServerRenderer,
} from 'shared/ReactFeatureFlags';

import {
REACT_FORWARD_REF_TYPE,
REACT_FRAGMENT_TYPE,
REACT_STRICT_MODE_TYPE,
REACT_ASYNC_MODE_TYPE,
REACT_PLACEHOLDER_TYPE,
REACT_PORTAL_TYPE,
REACT_PROFILER_TYPE,
REACT_PROVIDER_TYPE,
Expand Down Expand Up @@ -911,6 +916,27 @@ class ReactDOMServerRenderer {
this.stack.push(frame);
return '';
}
case REACT_PLACEHOLDER_TYPE: {
if (enableSuspenseServerRenderer) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if it's worth adding this to the existing renderer, even in this limited form. If we do, we should emit a warning so people know to migrate to the new one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The idea is to only enable this flag in the FB build for now since we're already using placeholders.

const nextChildren = toArray(
// Always use the fallback when synchronously rendering to string.
((nextChild: any): ReactElement).props.fallback,
);
const frame: Frame = {
type: null,
domNamespace: parentNamespace,
children: nextChildren,
childIndex: 0,
context: context,
footer: '',
};
if (__DEV__) {
((frame: any): FrameDev).debugElementStack = [];
}
this.stack.push(frame);
return '';
}
}
// eslint-disable-next-line-no-fallthrough
default:
break;
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/ReactFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ export const enableProfilerTimer = __PROFILE__;
// Track which interactions trigger each commit.
export const enableSchedulerTracking = __PROFILE__;

// Only used in www builds.
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
invariant(false, 'Not implemented.');
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fabric-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = __DEV__;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracking = __PROFILE__;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fabric-oss.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracking = __PROFILE__;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const enableUserTimingAPI = __DEV__;
export const warnAboutLegacyContextAPI = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracking = __PROFILE__;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const warnAboutDeprecatedLifecycles = false;
export const warnAboutLegacyContextAPI = false;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracking = __PROFILE__;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracking = __PROFILE__;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = false;
export const enableSchedulerTracking = false;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const warnAboutLegacyContextAPI = false;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
export const enableProfilerTimer = false;
export const enableSchedulerTracking = false;
export const enableSuspenseServerRenderer = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const {
debugRenderPhaseSideEffects,
debugRenderPhaseSideEffectsForStrictMode,
enableGetDerivedStateFromCatch,
enableSuspenseServerRenderer,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Normally we would export const enableSuspenseServerRenderer = true; instead of this line. That way it should be enabled in www client but I don't know how we plan on using this file on the server in www. Perhaps we should configure the server build to just inline the feature flags in the build instead? Since this one is most meant for experiments based on GKs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was planning on controlling this in www like so: https://fburl.com/diff/i1l0949n.

let me know if you'd rather just hard-code the value here.

replayFailedUnitOfWorkWithInvokeGuardedCallback,
warnAboutDeprecatedLifecycles,
} = require('ReactFeatureFlags');
Expand Down