-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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): more robust adapter zero-conf handling #38778
fix(gatsby): more robust adapter zero-conf handling #38778
Conversation
if (!dataToUse && tryGithubBeforeUnpkg) { | ||
dataToUse = await _fetchFile(GITHUB_ROOT, fileName) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optionally this uses github as source of content, to make it faster to deliver adapter version manifest changes to users - using unpkg require npm publish which currently is not the fastest process
import { satisfies } from "semver" | ||
import type { AdapterInit } from "./types" | ||
import { preferDefault } from "../../bootstrap/prefer-default" | ||
import { getLatestAdapters } from "../get-latest-gatsby-files" | ||
|
||
const getAdaptersCacheDir = (): string => join(process.cwd(), `.cache/adapters`) | ||
export const getAdaptersCacheDir = (): string => | ||
join(process.cwd(), `.cache/adapters`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit (not-blocking), could put the path in a constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably could, tho I vaguely remember a PR long time ago that moved similar path constructor from constant to function for some usecase where CWD changed after modules were imported.
It also was already a function and only change I did was to export it (to use in tests), so IMO there are is not enough benefits to mess with that (cost of running path.join
couple of times vs just once is something you can't even measure as variance will be much greater than perf difference), so I will just keep it as-is
* feat: prefer github for adapters version manifest * fix(gatsby): more robust adapter autoinstallation handling to prevent broken deploys * test: always allow using installed adapter version in e2e-tests/adapter (cherry picked from commit 7f08e7b)
* feat: prefer github for adapters version manifest * fix(gatsby): more robust adapter autoinstallation handling to prevent broken deploys * test: always allow using installed adapter version in e2e-tests/adapter (cherry picked from commit 7f08e7b) Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
Description
Current adapter handling is subpar and it's easy for build to skip using adapters which can result in broken deploys. For example if manually installed adapter by user is not matching the version expected to be used for current version of gatsby - gatsby would silently skip using any adapter and then redirect/rewrites, API functions, DSG/SSR would just not work.
This changes logic a bit so that:
Which means that situation described above wouldn't happen because we would discard incompatible adapter installed manually by user and instead autoinstall required version ourselves.
There were few other "improvements" made here:
gatsby-dev-cli
sets dev versions and semver matching, so added ability to provide custom adapter manifest via env var (and skip pulling manifest from github/npm/unpkg) and used that in our adapters e2e to always allow using dev version of adapter (aka code of adapter in tested branch / backport PR) - fix(gatsby): allow gatsby-adapter-netlify@">=1.0.0 <=1.0.3" for gatsby@<5.12.10 (#38758) #38762 is example of failure with some notes on why exactly it happenedDocumentation
Tests
Related Issues
#38752