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): resolve correct versions of packages with unpkg #33551

Merged
merged 3 commits into from
Oct 18, 2021

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Oct 15, 2021

Description

This PR fixes an issue with the incorrect detection of changed dependencies in the case when local dependencies are not installed.

Say we have this in project package.json and dependencies are not installed (so no node_modules folder):

"dependencies": {
  "gatsby": "next"
}

In this case gatsby-dev-cli attempts to load package details from unpkg to compare against dependencies in the monorepo:

// if package is not installed, we will do http GET request to
// unkpg to check if dependency in package published in public
// npm repository are different
// this allow us to not publish to local repository
// and save some time/work
try {
const response = await got(
`https://unpkg.com/${packageName}/package.json`
)

The problem is that packageName here will be gatsby and not gatsby@next. So it will always diff with dependencies of the latest version, regardless of the actual local version.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 15, 2021
@vladar vladar removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 15, 2021
`https://unpkg.com/${packageName}/package.json`
)
const version = getPackageVersion(packageName)
const url = `https://unpkg.com/${packageName}@${version}/package.json`
Copy link
Contributor

Choose a reason for hiding this comment

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

monorepoPKGjson.version = `${monorepoPKGjson.version}-dev-${versionPostFix}`

gatsby-dev might set to some version available only locally, so this unpkg would likely fail for those and fallback to using verdaccio which is fine I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This codepath is only active a) for the initial scan b) when there are no local dependencies (so no node_modules). If you have deps already installed with verdaccio, we won't even try to check unpkg.

So I think it should be fine

@vladar vladar merged commit 31d5a5e into master Oct 18, 2021
@vladar vladar deleted the gatsby-dev-cli-fix branch October 18, 2021 09:37
vladar added a commit that referenced this pull request Oct 18, 2021
…33551)

* fix(gatsby-dev-cli): resolve correct versions of packages with unpkg

* a bit more details in case of errors

* defaults for deps

(cherry picked from commit 31d5a5e)
vladar added a commit that referenced this pull request Oct 18, 2021
…33551) (#33570)

* fix(gatsby-dev-cli): resolve correct versions of packages with unpkg

* a bit more details in case of errors

* defaults for deps

(cherry picked from commit 31d5a5e)

Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com>
wardpeet pushed a commit that referenced this pull request Oct 18, 2021
…33551)

* fix(gatsby-dev-cli): resolve correct versions of packages with unpkg

* a bit more details in case of errors

* defaults for deps
wardpeet pushed a commit to herecydev/gatsby that referenced this pull request Oct 29, 2021
…atsbyjs#33551)

* fix(gatsby-dev-cli): resolve correct versions of packages with unpkg

* a bit more details in case of errors

* defaults for deps
axe312ger pushed a commit that referenced this pull request Nov 9, 2021
…33551)

* fix(gatsby-dev-cli): resolve correct versions of packages with unpkg

* a bit more details in case of errors

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

Successfully merging this pull request may close these issues.

2 participants