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

refactor[ci/build]: support FB_WWW_BROWSER_SCRIPT bundle type #27664

Closed
Closed
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
6 changes: 1 addition & 5 deletions .github/workflows/commit_artifacts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,6 @@ jobs:
mv build/oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js \
./compiled/facebook-www/eslint-plugin-react-hooks.js

# Copy unstable_server-external-runtime.js into facebook-www
mv build/oss-stable/react-dom/unstable_server-external-runtime.js \
./compiled/facebook-www/unstable_server-external-runtime.js

# Copy react-refresh-babel.development.js into babel-plugin-react-refresh
mv build/oss-experimental/react-refresh/cjs/react-refresh-babel.development.js \
./compiled/babel-plugin-react-refresh/index.js
Expand All @@ -148,7 +144,7 @@ jobs:
mkdir -p ${BASE_FOLDER}/react-native-github/Libraries/Renderer/
mkdir -p ${BASE_FOLDER}/RKJSModules/vendor/{scheduler,react,react-is,react-test-renderer}/

# Move React Native renderer
# Move React Native renderer
mv build/react-native/implementations/ $BASE_FOLDER/react-native-github/Libraries/Renderer/
mv build/react-native/shims/ $BASE_FOLDER/react-native-github/Libraries/Renderer/
mv build/facebook-react-native/scheduler/cjs/ $BASE_FOLDER/RKJSModules/vendor/scheduler/
Expand Down
9 changes: 7 additions & 2 deletions scripts/rollup/build-all-release-channels.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ function processStable(buildDir) {
const stats = fs.statSync(filePath);
if (!stats.isDirectory()) {
hash.update(fs.readFileSync(filePath));
fs.renameSync(filePath, filePath.replace('.js', '.classic.js'));

if (fileName !== 'unstable_server-external-runtime.js') {
fs.renameSync(filePath, filePath.replace('.js', '.classic.js'));
}
Comment on lines +172 to +174
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have access to bundle types at this point. Instead of relying on fileName I can parse bundles array and try finding everything what has bundleType FB_WWW_BROWSER_SCRIPT, if that seems like a better option

}
}
updatePlaceholderReactVersionInCompiledArtifacts(
Expand Down Expand Up @@ -242,7 +245,9 @@ function processExperimental(buildDir, version) {
const stats = fs.statSync(filePath);
if (!stats.isDirectory()) {
hash.update(fs.readFileSync(filePath));
fs.renameSync(filePath, filePath.replace('.js', '.modern.js'));
if (fileName !== 'unstable_server-external-runtime.js') {
fs.renameSync(filePath, filePath.replace('.js', '.modern.js'));
}
}
}
updatePlaceholderReactVersionInCompiledArtifacts(
Expand Down
34 changes: 26 additions & 8 deletions scripts/rollup/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const {
NODE_PROFILING,
BUN_DEV,
BUN_PROD,
FB_WWW_BROWSER_SCRIPT,
FB_WWW_DEV,
FB_WWW_PROD,
FB_WWW_PROFILING,
Expand Down Expand Up @@ -251,6 +252,7 @@ function getFormat(bundleType) {
case ESM_DEV:
case ESM_PROD:
return `es`;
case FB_WWW_BROWSER_SCRIPT:
case BROWSER_SCRIPT:
return `iife`;
}
Expand Down Expand Up @@ -280,6 +282,7 @@ function isProductionBundleType(bundleType) {
case RN_OSS_PROFILING:
case RN_FB_PROD:
case RN_FB_PROFILING:
case FB_WWW_BROWSER_SCRIPT:
case BROWSER_SCRIPT:
return true;
default:
Expand All @@ -304,6 +307,7 @@ function isProfilingBundleType(bundleType) {
case ESM_PROD:
case UMD_DEV:
case UMD_PROD:
case FB_WWW_BROWSER_SCRIPT:
case BROWSER_SCRIPT:
return false;
case FB_WWW_PROFILING:
Expand All @@ -322,10 +326,12 @@ function getBundleTypeFlags(bundleType) {
bundleType === UMD_DEV ||
bundleType === UMD_PROD ||
bundleType === UMD_PROFILING;
const isFBWWWBundle =
const isFBWWWBundleAndNotBrowserScript =
bundleType === FB_WWW_DEV ||
bundleType === FB_WWW_PROD ||
bundleType === FB_WWW_PROFILING;
const isFBWWWBundle =
isFBWWWBundleAndNotBrowserScript || bundleType === FB_WWW_BROWSER_SCRIPT;
const isRNBundle =
bundleType === RN_OSS_DEV ||
bundleType === RN_OSS_PROD ||
Expand All @@ -339,7 +345,10 @@ function getBundleTypeFlags(bundleType) {
bundleType === RN_FB_PROD ||
bundleType === RN_FB_PROFILING;

const shouldStayReadable = isFBWWWBundle || isRNBundle || forcePrettyOutput;
const isBrowserScript = FB_WWW_BROWSER_SCRIPT || BROWSER_SCRIPT;

const shouldStayReadable =
isFBWWWBundleAndNotBrowserScript || isRNBundle || forcePrettyOutput;

const shouldBundleDependencies =
bundleType === UMD_DEV ||
Expand All @@ -348,9 +357,11 @@ function getBundleTypeFlags(bundleType) {

return {
isUMDBundle,
isFBWWWBundleAndNotBrowserScript,
isFBWWWBundle,
isRNBundle,
isFBRNBundle,
isBrowserScript,
shouldBundleDependencies,
shouldStayReadable,
};
Expand Down Expand Up @@ -387,7 +398,8 @@ function getPlugins(
const isProduction = isProductionBundleType(bundleType);
const isProfiling = isProfilingBundleType(bundleType);

const {isUMDBundle, shouldStayReadable} = getBundleTypeFlags(bundleType);
const {isUMDBundle, shouldStayReadable, isFBWWWBundle, isBrowserScript} =
getBundleTypeFlags(bundleType);

const needsMinifiedByClosure = isProduction && bundleType !== ESM_PROD;

Expand All @@ -408,7 +420,8 @@ function getPlugins(
needsMinifiedByClosure &&
!isUMDBundle &&
!sourcemapPackageExcludes.includes(entry) &&
!shouldStayReadable;
!shouldStayReadable &&
bundleType !== FB_WWW_BROWSER_SCRIPT;

return [
// Keep dynamic imports as externals
Expand Down Expand Up @@ -494,11 +507,12 @@ function getPlugins(
language_out:
bundleType === NODE_ES2015
? 'ECMASCRIPT_2020'
: bundleType === BROWSER_SCRIPT
: isBrowserScript
? 'ECMASCRIPT5'
: 'ECMASCRIPT5_STRICT',
emit_use_strict:
bundleType !== BROWSER_SCRIPT &&
bundleType !== FB_WWW_BROWSER_SCRIPT &&
bundleType !== ESM_PROD &&
bundleType !== ESM_DEV,
env: 'CUSTOM',
Expand Down Expand Up @@ -697,12 +711,15 @@ async function createBundle(bundle, bundleType) {
const format = getFormat(bundleType);
const packageName = Packaging.getPackageName(bundle.entry);

const {isFBWWWBundle, isFBRNBundle, shouldBundleDependencies} =
getBundleTypeFlags(bundleType);
const {
isFBWWWBundleAndNotBrowserScript,
isFBRNBundle,
shouldBundleDependencies,
} = getBundleTypeFlags(bundleType);

let resolvedEntry = resolveEntryFork(
require.resolve(bundle.entry),
isFBWWWBundle || isFBRNBundle
isFBWWWBundleAndNotBrowserScript || isFBRNBundle
);

const peerGlobals = Modules.getPeerGlobals(bundle.externals, bundleType);
Expand Down Expand Up @@ -903,6 +920,7 @@ async function buildEverything() {
[bundle, NODE_PROFILING],
[bundle, BUN_DEV],
[bundle, BUN_PROD],
[bundle, FB_WWW_BROWSER_SCRIPT],
[bundle, FB_WWW_DEV],
[bundle, FB_WWW_PROD],
[bundle, FB_WWW_PROFILING],
Expand Down
5 changes: 4 additions & 1 deletion scripts/rollup/bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const bundleTypes = {
NODE_PROFILING: 'NODE_PROFILING',
BUN_DEV: 'BUN_DEV',
BUN_PROD: 'BUN_PROD',
FB_WWW_BROWSER_SCRIPT: 'FB_WWW_BROWSER_SCRIPT',
FB_WWW_DEV: 'FB_WWW_DEV',
FB_WWW_PROD: 'FB_WWW_PROD',
FB_WWW_PROFILING: 'FB_WWW_PROFILING',
Expand All @@ -43,6 +44,7 @@ const {
NODE_PROFILING,
BUN_DEV,
BUN_PROD,
FB_WWW_BROWSER_SCRIPT,
FB_WWW_DEV,
FB_WWW_PROD,
FB_WWW_PROFILING,
Expand Down Expand Up @@ -300,7 +302,7 @@ const bundles = [

/******* React DOM Fizz Server External Runtime *******/
{
bundleTypes: [BROWSER_SCRIPT],
bundleTypes: [BROWSER_SCRIPT, FB_WWW_BROWSER_SCRIPT],
moduleType: RENDERER,
entry: 'react-dom/unstable_server-external-runtime',
outputPath: 'unstable_server-external-runtime.js',
Expand Down Expand Up @@ -1130,6 +1132,7 @@ function getFilename(bundle, bundleType) {
case RN_FB_PROFILING:
case RN_OSS_PROFILING:
return `${globalName}-profiling.js`;
case FB_WWW_BROWSER_SCRIPT:
case BROWSER_SCRIPT:
return `${name}.js`;
}
Expand Down
18 changes: 18 additions & 0 deletions scripts/rollup/packaging.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const {
NODE_PROFILING,
BUN_DEV,
BUN_PROD,
FB_WWW_BROWSER_SCRIPT,
FB_WWW_DEV,
FB_WWW_PROD,
FB_WWW_PROFILING,
Expand Down Expand Up @@ -66,6 +67,23 @@ function getBundleOutputPath(bundle, bundleType, filename, packageName) {
case UMD_PROD:
case UMD_PROFILING:
return `build/node_modules/${packageName}/umd/${filename}`;
case FB_WWW_BROWSER_SCRIPT: {
// Bundles that are served as browser scripts need to be able to be sent
// straight to the browser with any additional bundling. We shouldn't use
// a module to re-export. Depending on how they are served, they also may
// not go through package.json module resolution, so we shouldn't rely on
// that either. We should consider the output path as part of the public
// contract, and explicitly specify its location within the package's
// directory structure.
const outputPath = bundle.outputPath;
if (!outputPath) {
throw new Error(
'Bundles with type FB_WWW_BROWSER_SCRIPT must specific an explicit ' +
'output path.'
);
}
return `build/facebook-www/${outputPath}`;
}
case FB_WWW_DEV:
case FB_WWW_PROD:
case FB_WWW_PROFILING:
Expand Down
3 changes: 2 additions & 1 deletion scripts/rollup/wrappers.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
NODE_PROFILING,
BUN_DEV,
BUN_PROD,
FB_WWW_BROWSER_SCRIPT,
FB_WWW_DEV,
FB_WWW_PROD,
FB_WWW_PROFILING,
Expand Down Expand Up @@ -442,7 +443,7 @@ function wrapBundle(
}
}

if (bundleType === BROWSER_SCRIPT) {
if (bundleType === BROWSER_SCRIPT || FB_WWW_BROWSER_SCRIPT) {
// Bundles of type BROWSER_SCRIPT get sent straight to the browser without
// additional processing. So we should exclude any extra wrapper comments.
return source;
Expand Down