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

Rollup build script --unsafe-partial flag is broken #22323

Closed
bvaughn opened this issue Sep 15, 2021 · 0 comments · Fixed by #22324
Closed

Rollup build script --unsafe-partial flag is broken #22323

bvaughn opened this issue Sep 15, 2021 · 0 comments · Fixed by #22324

Comments

@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2021

The following example command sequence will fail:

# Grab the latest build artifacts from CI
scripts/release/download-experimental-build.js --commit=main

# Rebuild only the local NODE bundle from the source of react-dom
# Leave all other artifacts untouched
yarn build --unsafe-partial --type=NODE_DEV react-dom/index

# Throws

The above sequence should work but instead fails with one of the two errors below:

Error: ENOENT: no such file or directory

Error: SyntaxError: Unexpected end of JSON input

This leaves the repo in a broken state (tests won't run, DevTools test shell will crash, etc).

These errors are thrown because this function prematurely resolves:

function asyncCopyTo(from, to) {
return asyncMkDirP(path.dirname(to)).then(
() =>
new Promise((resolve, reject) => {
ncp(from, to, error => {
if (error) {
// Wrap to have a useful stack trace.
reject(new Error(error));
return;
}
resolve();
});
})
);
}

Either before the package.json file has been copied, or after it has been copied (but while its contents are still empty).

This causes the subsequent read of package.json to throw:

function filterOutEntrypoints(name) {
// Remove entry point files that are not built in this configuration.
let jsonPath = `build/node_modules/${name}/package.json`;
let packageJSON = JSON.parse(readFileSync(jsonPath));

The error above can be verified by adding logging to the asyncTopyTo method:

function asyncCopyTo(from, to) {
  return asyncMkDirP(path.dirname(to)).then(
    () =>
      new Promise((resolve, reject) => {
        ncp(from, to, error => {
          if (error) {
            // Wrap to have a useful stack trace.
            reject(new Error(error));
            return;
          }
          if (to.includes('package.json')){
            console.log(`asyncCopyTo() -> ncp("${from}", "${to}") -> resolve`);

            // This line will either throw (no file) or print an empty string in many cases:
            console.log(require('fs').readFileSync(to).toString());
          }
          resolve();
        });
      })
  );
}

I'm able to "fix" this issue by introducing a small amount of delay, e.g.

diff --git a/scripts/rollup/packaging.js b/scripts/rollup/packaging.js
index eaf78959a6..5844d9116f 100644
--- a/scripts/rollup/packaging.js
+++ b/scripts/rollup/packaging.js
@@ -196,6 +196,10 @@ async function prepareNpmPackage(name) {
     ),
     asyncCopyTo(`packages/${name}/npm`, `build/node_modules/${name}`),
   ]);
+
+  // Wait for copied files to exist; asyncCopyTo() completes prematurely.
+  await new Promise(resolve => setTimeout(resolve, 100));
+
   filterOutEntrypoints(name);
   const tgzName = (
     await asyncExecuteCommand(`npm pack build/node_modules/${name}`)

But this feels pretty hacky and fragile.

Seems like this is perhaps related to AvianFlu/ncp#127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant