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

fix(gatsby-dev-cli): support workspaces in yarn@1.22 #24608

Merged
merged 2 commits into from
Jun 2, 2020

Conversation

pieh
Copy link
Contributor

@pieh pieh commented May 29, 2020

Description

When using yarn@1.22 it has different shape of yarn workspaces info --json command than previous versions of yarn - because gatsby-dev-cli relies on this output when running it in workspaces - we need some error catching / sanitization

@pieh pieh requested a review from a team as a code owner May 29, 2020 09:34
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 29, 2020
pvdz
pvdz previously approved these changes May 29, 2020
workspacesLayout = JSON.parse(JSON.parse(stdout).data)
} catch (e) {
/*
Yarn 1.22 doesn't output pure json - it has leading and trailing text:
Copy link
Contributor

Choose a reason for hiding this comment

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

:(
cc @arcanis

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Use the --silent flag to remove the header / footer from the output
  • I think in 1.x the --json flag will wrap the output into a "log" property which you probably don't want

In result, I think the command should be yarn workspaces info --silent in 1.x, and yarn workspaces list --json -v in 2.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the --silent flag to remove the header / footer from the output

Doesn't seem to work in 1.22 - in fact neither silent nor json really do - using yarn workspaces info, yarn workspaces info --json, yarn workspaces info --silent, yarn workspaces info --json --silent all output same thing:

$ yarn workspaces info --silent
yarn workspaces v1.22.0
warning package.json: No license field
{
  "z": {
    "location": "z",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": []
  },
  "y": {
    "location": "y",
    "workspaceDependencies": [],
    "mismatchedWorkspaceDependencies": []
  }
}
Done in 0.20s.

I think in 1.x the --json flag will wrap the output into a "log" property which you probably don't want

It does in <1.22 - it has { type: "log", data: "stringified-json" } (hence grabbing .data in JSON.parse(stdout).data), it doesnt in >=1.22 (or at least in 1.22.0).

In any case - this is really not looking to make this 100% reliable - I do want to explore using Yarn 2 with yarn link that uses portals protocol - like you did for "e2e pnp tests" (just gatsby-dev-cli is always on backlog and is never firest fire to work on, so it's just matter of not being able to spend more time on it)

Copy link
Contributor

Choose a reason for hiding this comment

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

It does in <1.22 - it has { type: "log", data: "stringified-json" } (hence grabbing .data in JSON.parse(stdout).data), it doesnt in >=1.22 (or at least in 1.22.0).

You're right, seems like a regression in the CLI parsing ... 😢

Will be good enough then 👍

if (sanitizedStdOut?.length >= 2) {
// pick content of first (and only) capturing group
const jsonString = sanitizedStdOut[1]
workspacesLayout = JSON.parse(jsonString)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably try/catch this too, emit a sensible error (for ourselves).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some try/catching and just log some details specific to this code path.

We will exit with generic error message below

@pieh pieh added type: bug An issue or pull request relating to a bug in Gatsby and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 29, 2020
@sidharthachatterjee sidharthachatterjee merged commit 40d241b into master Jun 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the gdc-yarn-1-22 branch June 2, 2020 11:21
axe312ger pushed a commit that referenced this pull request Jun 23, 2020
* fix(gatsby-dev-cli): support workspaces in yarn@1.22

* try/catch parsing "sanitized" output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants