Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

feat: add support for gatsby 5 #852

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, macOS-latest, windows-latest]
node-version: ['12.20.0', '*']
node-version: ['14.14.0', '*']
exclude:
- os: macOS-latest
node-version: '12.20.0'
node-version: '14.14.0'
- os: windows-latest
node-version: '12.20.0'
node-version: '14.14.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Node 12 was dropped.

fail-fast: false
runs-on: ${{ matrix.os }}
steps:
Expand Down
22 changes: 15 additions & 7 deletions src/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ const getContext = (context) => {
*/
export const listFrameworks = async function (context) {
const { pathExists, packageJson, packageJsonPath, nodeVersion } = getContext(context)
const { npmDependencies, scripts, runScriptCommand } = await getProjectInfo({
const { npmDependencies, npmDependenciesVersions, scripts, runScriptCommand } = await getProjectInfo({
pathExists,
packageJson,
packageJsonPath,
})
const frameworks = await pFilter(FRAMEWORKS, (framework) => usesFramework(framework, { pathExists, npmDependencies }))
const frameworks = await pFilter(FRAMEWORKS, (framework) =>
usesFramework(framework, { pathExists, npmDependencies, npmDependenciesVersions }),
)
const frameworkInfos = frameworks.map((framework) =>
getFrameworkInfo(framework, { scripts, runScriptCommand, nodeVersion }),
)
Expand All @@ -90,8 +92,12 @@ export const listFrameworks = async function (context) {
export const hasFramework = async function (frameworkId, context) {
const framework = getFrameworkById(frameworkId)
const { pathExists, packageJson, packageJsonPath } = getContext(context)
const { npmDependencies } = await getProjectInfo({ pathExists, packageJson, packageJsonPath })
const result = await usesFramework(framework, { pathExists, npmDependencies })
const { npmDependencies, npmDependenciesVersions } = await getProjectInfo({
pathExists,
packageJson,
packageJsonPath,
})
const result = await usesFramework(framework, { pathExists, npmDependencies, npmDependenciesVersions })
return result
}

Expand Down Expand Up @@ -131,11 +137,12 @@ const getFrameworkId = function ({ id }) {
}

const getProjectInfo = async function ({ pathExists, packageJson, packageJsonPath }) {
const { npmDependencies, scripts } = await getPackageJsonContent({
const { npmDependencies, npmDependenciesVersions, scripts } = getPackageJsonContent({
packageJson,
})
const runScriptCommand = await getRunScriptCommand({ pathExists, packageJsonPath })
return { npmDependencies, scripts, runScriptCommand }

return { npmDependencies, npmDependenciesVersions, scripts, runScriptCommand }
}

const getFrameworkInfo = function (
Expand All @@ -155,11 +162,12 @@ const getFrameworkInfo = function (
) {
const devCommands = getDevCommands({ frameworkDevCommand, scripts, runScriptCommand })
const recommendedPlugins = getPlugins(plugins, { nodeVersion })

return {
id,
name,
package: {
name: detect.npmDependencies[0],
name: typeof detect.npmDependencies[0] === 'string' ? detect.npmDependencies[0] : detect.npmDependencies[0]?.name,
version: 'unknown',
},
category,
Expand Down
29 changes: 22 additions & 7 deletions src/detect.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pLocate from 'p-locate'
import semver from 'semver'

// Checks if the project is using a specific framework:
// - if `framework.npmDependencies` is set, one of them must be present in the
Expand All @@ -14,20 +15,34 @@ export const usesFramework = async function (
configFiles,
},
},
{ pathExists, npmDependencies },
{ pathExists, npmDependencies, npmDependenciesVersions },
) {
return (
usesNpmDependencies(frameworkNpmDependencies, npmDependencies) &&
usesNpmDependencies(frameworkNpmDependencies, npmDependencies, npmDependenciesVersions) &&
lacksExcludedNpmDependencies(frameworkExcludedNpmDependencies, npmDependencies) &&
(await usesConfigFiles(configFiles, pathExists))
)
}

const usesNpmDependencies = function (frameworkNpmDependencies, npmDependencies) {
return (
frameworkNpmDependencies.length === 0 ||
frameworkNpmDependencies.some((frameworkNpmDependency) => npmDependencies.includes(frameworkNpmDependency))
)
const usesNpmDependencies = function (frameworkNpmDependencies, npmDependencies, npmDependenciesVersions) {
if (frameworkNpmDependencies.length === 0) {
return true
}

return frameworkNpmDependencies.some((frameworkNpmDependency) => {
if (typeof frameworkNpmDependency === 'string') {
return npmDependencies.includes(frameworkNpmDependency)
}

return (
npmDependencies.includes(frameworkNpmDependency.name) &&
(frameworkNpmDependency.version == null ||
semver.satisfies(
semver.minVersion(npmDependenciesVersions[frameworkNpmDependency.name]),
frameworkNpmDependency.version,
))
)
})
}

const lacksExcludedNpmDependencies = function (frameworkExcludedNpmDependencies, npmDependencies) {
Expand Down
2 changes: 1 addition & 1 deletion src/frameworks/gatsby.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"name": "Gatsby",
"category": "static_site_generator",
"detect": {
"npmDependencies": ["gatsby"],
"npmDependencies": [{ "name": "gatsby", "version": "<5.0.0" }],
"excludedNpmDependencies": [],
"configFiles": ["gatsby-config.js"]
},
Expand Down
37 changes: 37 additions & 0 deletions src/frameworks/gatsby5.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"id": "gatsby5",
"name": "Gatsby 5",
"category": "static_site_generator",
"detect": {
"npmDependencies": [{ "name": "gatsby", "version": ">=5.0.0" }],
"excludedNpmDependencies": [],
"configFiles": ["gatsby-config.js"]
},
"dev": {
"command": "gatsby develop",
"port": 8000,
"pollingStrategies": [{ "name": "TCP" }, { "name": "HTTP" }]
},
"build": {
"command": "gatsby build",
"directory": "public"
},
"staticAssetsDirectory": "static",
"env": {
"GATSBY_LOGGER": "yurnalist",
"GATSBY_PRECOMPILE_DEVELOP_FUNCTIONS": "true",
"AWS_LAMBDA_JS_RUNTIME": "nodejs18.x",
"NODE_VERSION": "18"
Copy link
Contributor Author

@danez danez Dec 14, 2022

Choose a reason for hiding this comment

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

These variables seem to only be used in the CLI and I'm not even certain about this. I haven't tested this because I think what we want cannot be achieved with setting NODE_VERSION. What we want is an explicit setting minimumRequiredNodeVersion, which I did not add in this PR, as this PR is only about splitting gatsby into two configs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of needing to create a new framework for each version. It feels hacky and hard to maintain. What happens when Gatsby 6 is released? Do we create a new one, or rename it to gatsby5plus? The approach of the plugins repo of allowing an array of versions, each with their own rules, seems to be better, though would be more of a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

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

To answer your question - I think you are right, and these are only used in ntl dev, so AWS_LAMBDA_JS_RUNTIME is also incorrect here.

Copy link
Contributor Author

@danez danez Dec 14, 2022

Choose a reason for hiding this comment

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

I just followed the pattern that existed for other frameworks as everything else would be a big change.
I think that it wouldn't even be a breaking change, just a lot of internal changes. The returned result could stay the same.

What I'm thinking of is something like this (simplified)

{
  "id": "gatsby5",
  "name": "Gatsby",
  "detect": {
    "npmDependencies": ["gatsby"],
    "excludedNpmDependencies": [],
    "configFiles": ["gatsby-config.js"]
  },
  "overrides": [
    {
      "detect": {
        "npmDependencies": [{name: "gatsby", version: ">99"}],
      },
      "dev": {
        "port": 1111,
      },
    }
  ],
  "dev": {
    "port": 8888,
  },
}

So in this example it would change the dev port if a certain version of the framework is used, but inherit everything else from the initial config. The structure in the processed end result would be exactly the same, at least for the framework detection.

},
"logo": {
"default": "/logos/gatsby/default.svg",
"light": "/logos/gatsby/light.svg",
"dark": "/logos/gatsby/dark.svg"
},
"plugins": [
{
"packageName": "@netlify/plugin-gatsby",
"condition": { "minNodeVersion": "12.13.0" }
}
]
}
1 change: 1 addition & 0 deletions src/frameworks/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const FRAMEWORK_NAMES = [
'docusaurus-v2',
'eleventy',
'gatsby',
'gatsby5',
'gridsome',
'hexo',
'hugo',
Expand Down
8 changes: 7 additions & 1 deletion src/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,21 @@ export const getPackageJsonContent = function ({ packageJson }) {
}

const npmDependencies = getNpmDependencies(packageJson)
const npmDependenciesVersions = getNpmDependenciesVersions(packageJson)
const scripts = getScripts(packageJson)
return { npmDependencies, scripts }
return { npmDependencies, npmDependenciesVersions, scripts }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm exporting all the versions as Map<dep-name, version> in addition to the array of dependency names, because otherwise we would have todo Object.keys() a lot everywhere, and I wasn't sure about if the performance would suffer.

It is not ideal to kinda return the same info in different formats, but I think that is the compromise here.

}

// Retrieve `package.json` `dependencies` and `devDependencies` names
const getNpmDependencies = function ({ dependencies, devDependencies }) {
return [...getObjectKeys(dependencies), ...getObjectKeys(devDependencies)]
}

// Retrieve `package.json` `dependencies` and `devDependencies` versions
const getNpmDependenciesVersions = function ({ dependencies, devDependencies }) {
return { ...devDependencies, ...dependencies }
}

const getObjectKeys = function (value) {
if (!isPlainObj(value)) {
return []
Expand Down
12 changes: 12 additions & 0 deletions test/detect.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,15 @@ if (nodeVersion === 'v8.3.0') {
t.is(frameworks[0].plugins.length, 0)
})
}

test('Should detect specific version of dependencies', async (t) => {
const frameworks = await getFrameworks('gatsby5')
t.is(frameworks.length, 1)
t.is(frameworks[0].id, 'gatsby5')
})

test('Should not detect specific version of dependencies if version does not match', async (t) => {
const frameworks = await getFrameworks('gatsby4')
t.is(frameworks.length, 1)
t.is(frameworks[0].id, 'gatsby')
})
Empty file.
7 changes: 7 additions & 0 deletions test/fixtures/gatsby4/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "test",
"version": "1.0.0",
"dependencies": {
"gatsby": "4"
}
}
Empty file.
7 changes: 7 additions & 0 deletions test/fixtures/gatsby5/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "test",
"version": "1.0.0",
"dependencies": {
"gatsby": "5"
}
}
12 changes: 11 additions & 1 deletion test/frameworks.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,17 @@ const FRAMEWORK_JSON_SCHEMA = {
properties: {
npmDependencies: {
type: 'array',
items: { type: 'string', minLength: 1 },
items: {
anyOf: [
{ type: 'string', minLength: 1 },
{
type: 'object',
required: ['name'],
additionalProperties: false,
properties: { name: { type: 'string', minLength: 1 }, version: { type: 'string', minLength: 1 } },
},
],
},
},
excludedNpmDependencies: {
type: 'array',
Expand Down