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

[build-utils] Fix Yarn 2 install argument order #4444

Merged
merged 6 commits into from May 27, 2020

Conversation

SagnikPradhan
Copy link
Contributor

Fix for Yarn Version 2 install error. Positioned the arguments array in proper order #4436

Fix for Yarn Version 2. Positioned the arguments array in proper order
@MarkLyck
Copy link

@styfle @AndyBitz 🙏

@@ -291,7 +291,7 @@ export async function runNpmInstall(
} else {
opts.prettyCommand = 'yarn install';
command = 'yarn';
commandArgs = args.concat(['install', '--ignore-engines']);
commandArgs = ['install', ...args, '--ignore-engines'];
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Please add a test fixture under now-build-utils/test/fixtures so we can see what this is fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So from what I understand I have to add a sample project with yarn 2 there right?

Copy link
Member

@styfle styfle May 22, 2020

Choose a reason for hiding this comment

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

Yes, you would add a project that would fail with the current behavior but is fixed with your change to the arguments.

I'm guessing you have yarn 2 as a dev dependency?

The original description doesn't explain how to reproduce the error or what the error says.

Copy link
Member

@styfle styfle May 22, 2020

Choose a reason for hiding this comment

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

Update: I just saw #4436.

You can add the code from your repo as a test fixture.

But it doesn't install yarn 2?

Choose a reason for hiding this comment

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

@styfle Yarn 2 works in an interesting way.

The version that's installed is actually yarn 1.22 (usually this is installed globally not as a dev dependency), but you set it to use Yarn 2 in the project level by running the command yarn set version berry which then sets up that project to use the latest yarn version.

I think it should still be reproducible in a sample project I think, due to yarn installing Yarn 2 in the .yarn folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @MarkLyck is kinda right, globally any version can be installed, but then you can also have an another local version installed in the .yarn folder and point to it through the .yarnrc (in case of v2 .yarnrc.yml) file. This behavior is not specific to yarn v2, you can try doing yarn set version latest in an non v2 project too.

Thus, generally, projects using v2 have it installed locally, with the help of yarn set version berry.

And Alright will be spinning up the default demo project with yarn v2!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@styfle Added a Template Svelte project with yarn v2

@styfle styfle changed the title Fix Yarn Version 2 install errors on Build Process [build-utils] Fix Yarn Version 2 install errors on Build Process May 21, 2020
@phuctm97
Copy link

phuctm97 commented May 24, 2020

How this PR is going, guys? I really need it. Anything I can help? I see some checks are failing.

@SagnikPradhan
Copy link
Contributor Author

How this PR is going, guys? I really need it. Anything I can help? I see some checks are failing.

Most of the tests are failing because of being a 3rd Party PR. Although not sure about why Unit / Unit (macos-latest, 10) (pull_request) failed, maybe because of Github acting weird? IDK

Anyway waiting for @styfle and @AndyBitz for review

@SagnikPradhan
Copy link
Contributor Author

Can someone look into this? It is a fairly small PR, should not take more than 20 Minutes. The Fixture is the default template project too.

@styfle styfle linked an issue May 27, 2020 that may be closed by this pull request
@styfle
Copy link
Member

styfle commented May 27, 2020

I ran the test locally and it is not passing.

  ● Should build "19-yarn-v2"

    expect(received).resolves.toBeDefined()

    Received promise rejected instead of resolved
    Rejected to value: [SyntaxError: JSON5: invalid character 'u' at 1:1]

We need to add a now.json or vercel.json file to the test fixture.

@styfle
Copy link
Member

styfle commented May 27, 2020

When I added vercel.json with the following:

{
  "version": 2,
  "builds": [
    { "src": "package.json", "use": "@vercel/static-build" }
  ],
  "probes": [
    { "path": "/", "mustContain": "Svelte app" }
  ]
}

I get an error:

Running "yarn run build"
Unknown Syntax Error: Command not found;

Is yarn run build not supported anymore?


Update: This too needed to move the --ignore-engines flag to the end.

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@styfle styfle changed the title [build-utils] Fix Yarn Version 2 install errors on Build Process [build-utils] Fix Yarn 2 install argument order May 27, 2020
@styfle styfle merged commit dd2399f into vercel:master May 27, 2020
@SagnikPradhan
Copy link
Contributor Author

I still seem to get the same error, is the master branch deployed?

styfle added a commit that referenced this pull request May 27, 2020
Follow up to #4444 that makes sure we run Next.js tests.

This `--ignore-engines` flag was originally added in vercel/now-builders#463 as a workaround. We can remove it to make sure the version of Node selected will work with the dependencies.

Removing the flag also makes sure that Yarn 2 will work properly, see #4444.
@phuctm97
Copy link

When will this get deployed to Vercel production, guys?

@SagnikPradhan SagnikPradhan deleted the fix/yarn-v2-build-error branch May 28, 2020 04:26
@styfle
Copy link
Member

styfle commented May 28, 2020

This change is available on the canary channel npm install -g vercel@canary

@MarkLyck
Copy link

@styfle when will it be deployed to production? I solely use the GitHub app to deploy our sites, I don't think there's any way I can point that to canary?

@styfle
Copy link
Member

styfle commented May 28, 2020

It will be available in the next stable release in a few weeks or so

@SagnikPradhan
Copy link
Contributor Author

@styfle Is there a way we could use canary builds during build process or deployments?

@styfle
Copy link
Member

styfle commented May 29, 2020

Git deployments don't have a way to opt-in to canary builds yet but we will add a flag in the future

@SagnikPradhan
Copy link
Contributor Author

@styfle Is there any other way to deploy? Other than git deployments?

@styfle
Copy link
Member

styfle commented May 29, 2020

Yes with Vercel CLI. See my previous comment #4444 (comment)

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

Successfully merging this pull request may close these issues.

Yarn version 2 install errors out
4 participants