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

[Flight] Wire up bundler configs #18334

Merged
merged 1 commit into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ module.exports = {
nativeFabricUIManager: true,
},
},
{
files: ['packages/react-flight-dom-webpack/**/*.js'],
globals: {
'__webpack_chunk_load__': true,
'__webpack_require__': true,
},
},
],

globals: {
Expand Down
7 changes: 7 additions & 0 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@

import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';

// import type {ModuleMetaData} from './ReactFlightClientHostConfig';

// import {
// preloadModule,
// requireModule,
// } from './ReactFlightClientHostConfig';

export type ReactModelRoot<T> = {|
model: T,
|};
Expand Down
30 changes: 30 additions & 0 deletions packages/react-client/src/ReactFlightClientHostConfigNoStream.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export type StringDecoder = void;

export const supportsBinaryStreams = false;

export function createStringDecoder(): void {
throw new Error('Should never be called');
}

export function readPartialStringChunk(
decoder: StringDecoder,
buffer: Uint8Array,
): string {
throw new Error('Should never be called');
}

export function readFinalStringChunk(
decoder: StringDecoder,
buffer: Uint8Array,
): string {
throw new Error('Should never be called');
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
// really an argument to a top-level wrapping function.

declare var $$$hostConfig: any;

export opaque type ModuleMetaData = mixed; // eslint-disable-line no-undef
export const preloadModule = $$$hostConfig.preloadModule;
export const requireModule = $$$hostConfig.requireModule;

export opaque type Source = mixed; // eslint-disable-line no-undef
export opaque type StringDecoder = mixed; // eslint-disable-line no-undef

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
*/

export * from 'react-client/src/ReactFlightClientHostConfigBrowser';
export * from 'react-flight-dom-webpack/src/ReactFlightClientWebpackBundlerConfig';
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
*/

export * from 'react-flight-dom-relay/src/ReactFlightDOMRelayClientHostConfig';
export * from '../ReactFlightClientHostConfigNoStream';
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
*/

export * from 'react-client/src/ReactFlightClientHostConfigBrowser';
export * from 'react-flight-dom-webpack/src/ReactFlightClientWebpackBundlerConfig';
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,9 @@
* @flow
*/

export type StringDecoder = void;
export {
preloadModule,
requireModule,
} from 'ReactFlightDOMRelayClientIntegration';

export const supportsBinaryStreams = false;

export function createStringDecoder(): void {
throw new Error('Should never be called');
}

export function readPartialStringChunk(
decoder: StringDecoder,
buffer: Uint8Array,
): string {
throw new Error('Should never be called');
}

export function readFinalStringChunk(
decoder: StringDecoder,
buffer: Uint8Array,
): string {
throw new Error('Should never be called');
}
export type {ModuleMetaData} from 'ReactFlightDOMRelayClientIntegration';
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {Destination} from './ReactFlightDOMRelayServerHostConfig';
import {createRequest, startWork} from 'react-server/src/ReactFlightServer';

function render(model: ReactModel, destination: Destination): void {
let request = createRequest(model, destination);
let request = createRequest(model, destination, undefined);
startWork(request);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,34 @@

import type {Request, ReactModel} from 'react-server/src/ReactFlightServer';

import type {Destination} from 'ReactFlightDOMRelayServerIntegration';
import type {
Destination,
ModuleReference,
ModuleMetaData,
} from 'ReactFlightDOMRelayServerIntegration';

import {resolveModelToJSON} from 'react-server/src/ReactFlightServer';

import {emitModel, emitError} from 'ReactFlightDOMRelayServerIntegration';

export type {Destination} from 'ReactFlightDOMRelayServerIntegration';
import {
emitModel,
emitError,
resolveResourceMetaData,
} from 'ReactFlightDOMRelayServerIntegration';

export type {
Destination,
ModuleReference,
ModuleMetaData,
} from 'ReactFlightDOMRelayServerIntegration';

export type BundlerConfig = void;

export function resolveModuleMetaData(
config: BundlerConfig,
resource: ModuleReference,
): ModuleMetaData {
return resolveResourceMetaData(resource);
}

type JSONValue =
| string
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

function getFakeModule() {
return function FakeModule(props, data) {
return data;
};
}

const ReactFlightDOMRelayClientIntegration = {
preloadModule(jsResource) {
return null;
},
requireModule(jsResource) {
return getFakeModule();
},
};

module.exports = ReactFlightDOMRelayClientIntegration;
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const ReactFlightDOMRelayServerIntegration = {
});
},
close(destination) {},
resolveResourceMetaData(resource) {
return resource;
},
};

module.exports = ReactFlightDOMRelayServerIntegration;
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export type ModuleMetaData = {
id: string,
chunks: Array<string>,
Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
id: string,
chunks: Array<string>,
id: string | number,
chunks: Array<string | number>,

};

type Thenable = {
then(resolve: () => mixed, reject: (mixed) => mixed): mixed,
...
};

// The chunk cache contains all the chunks we've preloaded so far.
// If they're still pending they're a thenable. This map also exists
// in Webpack but unfortunately it's not exposed so we have to
// replicate it in user space. null means that it has already loaded.
const chunkCache: Map<string, null | Thenable> = new Map();
Copy link
Collaborator Author

@sebmarkbage sebmarkbage Mar 18, 2020

Choose a reason for hiding this comment

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

@sokra In this approach, I'll always call __webpack_chunk_load__ eagerly so there's always an opportunity for it to kick off async requests as a result. Always getting the same Promise back would be helpful since I wouldn't need to cache it. However, I also need to know if they've all been resolved so I know it's safe to call require.

This means I have to replicate this information in a user space map anyway.

It would be nice if there was a way to call __webpack_chunk_load__, so that there's something that spawns async work to kick off but synchronously know if it's already safe to call __webpack_require__. It's fine that we're forced to call __webpack_chunk_load__ and even that it's always async at the first call, but if we have already done so in the past, it would be nice to query the webpack cache rather than our own.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const chunkCache: Map<string, null | Thenable> = new Map();
const chunkCache: Map<string | number, null | Thenable> = new Map();

I consider adding an extra argument to __webpack_chunk_load__ which make it return null instead of a resolved Promise when the chunk is already loaded.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would be great! Thank you.


// Returning null means that all dependencies are fulfilled and we
// can synchronously require the module now. A thenable is returned
// that when resolved, means we can try again.
export function preloadModule(moduleData: ModuleMetaData): null | Thenable {
let moduleEntry = require.cache[moduleData.id];
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically using require.cache and ESM is kind of illegal. It works in webpack in "automatic" JS mode (kind of mixed CJS and ESM), but require.cache is not available in strict ESM mode. At least it works for webpack <= 5.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have you considered exposing a Webpack specific helper to do something similar to this?

In this case I could just get rid of it since it’s just an optimization to avoid having to check many maps.

It might be nice to know for scheduling purposes though. E.g. we try to be very particular about when we call __webpack_require__ since that can kick off a lot of synchronous work. So it’s nice to be able to know if the call will be free or hundreds of ms.

if (moduleEntry) {
// Fast exit if this module has already been loaded.
return null;
}
let chunks = moduleData.chunks;
let anyRemainingThenable = null;
for (let i = 0; i < chunks.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This code snippet doesn't work for more than 1 chunks, as anyRemainingThenable is overwritten in each iteration.

You probably want to use Promise.all here?

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 will be recalled each time a new chunk resolves which will then get the next one and so on. However in practice it most likely won’t get as far as even retrying the second time until everything has been resolved.

So in practice we get one promise, then wait for it, schedule work to try again, in the meantime all of them resolve and then we try again just to confirm they all resolved. If not we’ll do the whole thing again.

let chunkId = chunks[i];
let entry = chunkCache.get(chunkId);
if (entry === undefined) {
anyRemainingThenable = __webpack_chunk_load__(chunkId);
chunkCache.set(chunkId, anyRemainingThenable);
anyRemainingThenable.then(chunkCache.set.bind(chunkCache, chunkId, null));
} else if (entry !== null) {
anyRemainingThenable = entry;
}
}
return anyRemainingThenable;
}

export function requireModule<T>(moduleData: ModuleMetaData): T {
return __webpack_require__(moduleData.id).default;
Copy link
Contributor

Choose a reason for hiding this comment

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

In production mode webpack may mangles exports names if certain conditions are met. This means .default might be .n or something else.

It depends on how the module is used and how you get the id. You could opt-out from these optimization, but the better idea is probably to add the mangled export name to the ModuleMetaData. I guess you extract this meta data with a webpack plugin. Here you can also access the final name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Yea that sounds like a good idea.

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@
*/

import type {ReactModel} from 'react-server/src/ReactFlightServer';
import type {BundlerConfig} from './ReactFlightServerWebpackBundlerConfig';

import {
createRequest,
startWork,
startFlowing,
} from 'react-server/src/ReactFlightServer';

function renderToReadableStream(model: ReactModel): ReadableStream {
function renderToReadableStream(
model: ReactModel,
webpackMap: BundlerConfig,
): ReadableStream {
let request;
return new ReadableStream({
start(controller) {
request = createRequest(model, controller);
request = createRequest(model, controller, webpackMap);
startWork(request);
},
pull(controller) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {ReactModel} from 'react-server/src/ReactFlightServer';
import type {BundlerConfig} from './ReactFlightServerWebpackBundlerConfig';
import type {Writable} from 'stream';

import {
Expand All @@ -20,8 +21,12 @@ function createDrainHandler(destination, request) {
return () => startFlowing(request);
}

function pipeToNodeWritable(model: ReactModel, destination: Writable): void {
let request = createRequest(model, destination);
function pipeToNodeWritable(
model: ReactModel,
destination: Writable,
webpackMap: BundlerConfig,
): void {
let request = createRequest(model, destination, webpackMap);
destination.on('drain', createDrainHandler(destination, request));
startWork(request);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

type WebpackMap = {
[filename: string]: ModuleMetaData,
};

export type BundlerConfig = WebpackMap;

export type ModuleReference = string;

export type ModuleMetaData = {
id: string,
chunks: Array<string>,
};

export function resolveModuleMetaData(
config: BundlerConfig,
modulePath: ModuleReference,
): ModuleMetaData {
return config[modulePath];
}
7 changes: 6 additions & 1 deletion packages/react-noop-renderer/src/ReactNoopFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ const ReactNoopFlightServer = ReactFlightServer({

function render(model: ReactModel): Destination {
let destination: Destination = [];
let request = ReactNoopFlightServer.createRequest(model, destination);
let bundlerConfig = undefined;
let request = ReactNoopFlightServer.createRequest(
model,
destination,
bundlerConfig,
);
ReactNoopFlightServer.startWork(request);
return destination;
}
Expand Down
12 changes: 11 additions & 1 deletion packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
* @flow
*/

import type {Destination, Chunk} from './ReactFlightServerConfig';
import type {
Destination,
Chunk,
BundlerConfig,
// ModuleReference,
// ModuleMetaData,
} from './ReactFlightServerConfig';

import {
scheduleWork,
Expand All @@ -18,6 +24,7 @@ import {
close,
processModelChunk,
processErrorChunk,
// resolveModuleMetaData,
} from './ReactFlightServerConfig';

import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
Expand Down Expand Up @@ -49,6 +56,7 @@ type Segment = {

export type Request = {
destination: Destination,
bundlerConfig: BundlerConfig,
nextChunkId: number,
pendingChunks: number,
pingedSegments: Array<Segment>,
Expand All @@ -61,10 +69,12 @@ export type Request = {
export function createRequest(
model: ReactModel,
destination: Destination,
bundlerConfig: BundlerConfig,
): Request {
let pingedSegments = [];
let request = {
destination,
bundlerConfig,
nextChunkId: 0,
pendingChunks: 0,
pingedSegments: pingedSegments,
Expand Down
Loading