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

Spurious error when installing nightly build #1541

Closed
athomann opened this issue Jan 11, 2022 · 12 comments
Closed

Spurious error when installing nightly build #1541

athomann opened this issue Jan 11, 2022 · 12 comments
Labels
bug Something isn't working build / infra

Comments

@athomann
Copy link

I'm attempting to install the nightly build via yarn but the command fails with this error.

error /Users/home/dev/proj/web/node_modules/recoil: Command failed.
Exit code: 1
Command: cd ./node_modules/gen-flow-files && yarn install && yarn build
Arguments: 
@athomann
Copy link
Author

Looks like the error is it can't find gen-flow-files directory. Maybe the command needs to yarn install before triggering gen-flow-files?

@athomann
Copy link
Author

Was able to get around the error with --ignore-scripts

@drarmstr drarmstr changed the title Error when installing nightly build Spurious error when installing nightly build Jan 11, 2022
@drarmstr
Copy link
Contributor

That post-build script is used to generate the Flow files and should already be done as part of the original install when the nightly branch was built. So, this error shouldn't be a problem and ignoring it should be safe. Would you be up to help adjust scripts/deploy_nightly_build.js to clean this up?

Or even better if we could find a better alternative to using/patching gen-flow-files...

@drarmstr drarmstr added build / infra help wanted Extra attention is needed labels Jan 11, 2022
@gurkerl83
Copy link

gurkerl83 commented Jan 11, 2022

I think cleaning up might result in removing the gen-flow-files dependency at all, at least how it is integrated right now targeting a git repo at the source level, not a bundle after a build similar to what recoils nightly branch does.

@drarmstr It seems you forked the gen-flow-files repo, made some changes with babel, and pushed the commits.

My question is about the requirement to use the dependency; respectively, I`m questioning how flow files are spit out by this tool, corresponding to the recoils source folder structure. A way to prevent this may be a flow plugin for the rollup bundler.

One thing to note is that the generated flow files are distributed for the commonJS format only. See e.g.

https://github.com/facebookexperimental/Recoil/tree/nightly/cjs/core

I don`t have experience with flow files and how to handle them. Thx!

Update: Adding the following conversation for reference.
facebook/flow#5871 (comment)

@drarmstr
Copy link
Contributor

Yes exactly, the proper cleanup is probably migrating gen-flow-files to flow-copy-source or something else (cc @mondaychen). The reason we are using a forked version of gen-flow-files is just because it is no longer maintained and we needed an update to support newer syntax with a newer Babel.

@SmallHollow
Copy link

SmallHollow commented Jan 29, 2022

@drarmstr I get this same error while trying install Recoil 0.6.0 (npm install recoil@0.6.0 with npm 8.1.2). With --ignore-scripts the install works.

@salvoravida
Copy link
Contributor

@drarmstr I get this same error while trying install Recoil 0.6.0 (npm install recoil@0.6.0 with npm 8.1.2). With --ignore-scripts the install works.

same here

@gurkerl83
Copy link

The error can also be suppressed in yarn. Here is the necessary configuration in package.json

"dependenciesMeta": {
   "recoil": {
      "built": false
   }
},

Tested with Yarn 3.

If an app is deployed with NextJs / Vercel, presumably other providers are also affected, this configuration must be included, otherwise a deployment will fail.

@jonbnewman
Copy link

This error is showing up with the latest general release v0.6.0 @drarmstr

@salvoravida
Copy link
Contributor

if it can help, meanwhile, I have published a clone without a "post-install" script, use it with yarn resolutions

  "resolutions": {
    "recoil": "yarn:@salvoravida/recoil"
  },

@gurkerl83
Copy link

gurkerl83 commented Jan 29, 2022

@drarmstr Due to the large number of downloads (>77k) of the latest stable version 0.5.2, it might be appropriate to temporarily remove the post-install step and trigger new deployment on NPM.

As described further above the incorrect build step tries to generate flow files after the download performed by a package manager.

These are generated like the bundles themselves before a deployment, and shipped on NPM as well. A repeated build with missing sources on the client leads to the error.

@drarmstr drarmstr linked a pull request Jan 29, 2022 that will close this issue
@drarmstr drarmstr added bug Something isn't working and removed help wanted Extra attention is needed labels Jan 29, 2022
@drarmstr
Copy link
Contributor

Fixed with #1577 and 0.6.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build / infra
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants