Skip to content

refactor: move into npm workspace #1304

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

Merged
merged 9 commits into from
Apr 19, 2022
Merged
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
16 changes: 9 additions & 7 deletions .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,27 @@ jobs:
id: release
with:
token: ${{ steps.get-token.outputs.token }}
release-type: node
package-name: '@netlify/plugin-nextjs'
command: manifest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're changing to manifest releases because the package is in a subdirectory

- uses: actions/checkout@v2
if: ${{ steps.release.outputs.release_created }}
if: ${{ steps.release.outputs.releases_created }}
- uses: actions/setup-node@v2
with:
node-version: '*'
cache: 'npm'
check-latest: true
registry-url: 'https://registry.npmjs.org'
if: ${{ steps.release.outputs.release_created }}
if: ${{ steps.release.outputs.releases_created }}
- name: Install dependencies
run: npm ci
if: ${{ steps.release.outputs.release_created }}
working-directory: plugin
if: ${{ steps.release.outputs.releases_created }}
- run: npm publish
if: ${{ steps.release.outputs.release_created }}
working-directory: plugin
if: ${{ steps.release.outputs.releases_created }}
env:
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}
- uses: netlify/submit-build-plugin-action@v1
if: ${{ steps.release.outputs.release_created }}
if: ${{ steps.release.outputs.releases_created }}
with:
github-token: ${{ steps.get-token.outputs.token }}
package-json-dir: plugin
22 changes: 18 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,20 @@ jobs:

steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
- name: Installing with latest Node.js
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 is so we can use workspaces, which require node 16, but still run tests in node 12

uses: actions/setup-node@v2
if: "${{ matrix.node-version != '*' }}"
with:
node-version: ${{ matrix.node-version }}
node-version: '*'
check-latest: true
- name: NPM Install
run: npm ci
- name: Switching to Node.js ${{ matrix.node-version }} to run tests
uses: actions/setup-node@v2
if: "${{ matrix.node-version != '*' }}"
with:
node-version: ${{ matrix.node-version }}
check-latest: true
- name: Linting
run: npm run format:ci
if: "${{ matrix.node-version == '*' }}"
Expand All @@ -55,14 +62,21 @@ jobs:
if: github.ref_name == 'main'
steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
- name: Installing with latest Node.js
uses: actions/setup-node@v2
if: "${{ matrix.node-version != '*' }}"
with:
node-version: ${{ matrix.node-version }}
node-version: '*'
check-latest: true
- name: NPM Install
run: npm ci
- name: Install Next.js Canary
run: npm install -D next@canary --legacy-peer-deps
- name: Switching to Node.js ${{ matrix.node-version }} to run tests
uses: actions/setup-node@v2
if: "${{ matrix.node-version != '*' }}"
with:
node-version: ${{ matrix.node-version }}
check-latest: true
- name: Run tests against next@canary
run: npm test
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
17.9.0

Choose a reason for hiding this comment

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

Should this perhaps be set to v16 so that this is on an LTS version of node? Or is there something about v17 that we want for these changes?

Depending on when it's looking like these changes will be merged and released we might be able to even go up to the newest LTS version (v18) which will be released on the April 19th.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I did this because I thought it needed to be 17 for workspaces to work properly, but I think 16 should be ok. Good point re 18 though. We should probably migrate later.

I still need to work out the best way to run the node 12 tests though, because right now they're failing as they don't support workspaces at all.

2 changes: 2 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ logs
*.log
npm-debug.log*

package.json

# Dependency directories
node_modules

Expand Down
3 changes: 3 additions & 0 deletions .release-please-manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugin": "4.3.2"
}
5 changes: 2 additions & 3 deletions demos/base-path/netlify.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ CYPRESS_CACHE_FOLDER = "../node_modules/.CypressBinary"
framework = "#static"

[[plugins]]
package = "./local-plugin"
package = "../plugin-wrapper/"

[[plugins]]
package = "@netlify/plugin-local-install-core"

# [[context.deploy-preview.plugins]]
# package = "netlify-plugin-cypress"
# package = "netlify-plugin-cypress"
22 changes: 22 additions & 0 deletions demos/base-path/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "base-path-demo",
"version": "1.0.0",
"description": "",
"devDependencies": {
"@netlify/plugin-nextjs": "*"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "git+https://github.com/netlify/netlify-plugin-nextjs.git"
},
"author": "",
"private": true,
"license": "MIT",
"bugs": {
"url": "https://github.com/netlify/netlify-plugin-nextjs/issues"
},
"homepage": "https://github.com/netlify/netlify-plugin-nextjs#readme"
}
1 change: 0 additions & 1 deletion demos/default/local-plugin/index.js

This file was deleted.

1 change: 0 additions & 1 deletion demos/default/local-plugin/manifest.yml

This file was deleted.

11 changes: 0 additions & 11 deletions demos/default/local-plugin/package.json

This file was deleted.

8 changes: 5 additions & 3 deletions demos/default/netlify.toml
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
[build]
command = "next build"
publish = ".next"
ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF ../../"
ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../../plugin"

[build.environment]
# cache Cypress binary in local "node_modules" folder
# so Netlify caches it
CYPRESS_CACHE_FOLDER = "../node_modules/.CypressBinary"
# set TERM variable for terminal output
TERM = "xterm"
NODE_VERSION = "17"

[dev]
framework = "#static"

[[plugins]]
package = "./local-plugin"
package = "../plugin-wrapper/"

# This is a fake plugin, that makes it run npm install
[[plugins]]
package = "@netlify/plugin-local-install-core"
package = "@netlify/plugin-local-install-core"
26 changes: 26 additions & 0 deletions demos/default/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"name": "default-demo",
"version": "1.0.0",
"private": true,
"description": "",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "git+https://github.com/netlify/netlify-plugin-nextjs.git"
},
"author": "",
"license": "MIT",
"bugs": {
"url": "https://github.com/netlify/netlify-plugin-nextjs/issues"
},
"homepage": "https://github.com/netlify/netlify-plugin-nextjs#readme",
"dependencies": {
"@reach/dialog": "^0.16.2",
"@reach/visually-hidden": "^0.16.0"
},
"devDependencies": {
"@netlify/plugin-nextjs": "*"
}
}
30 changes: 25 additions & 5 deletions demos/default/pages/env.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,39 @@
import { useEffect, useState } from 'react'

/**
* https://nextjs.org/docs/basic-features/environment-variables
*/
function EnvTest() {
const [showEnv, setShowEnv] = useState(false)
useEffect(() => setShowEnv(true), [])
return (
<div>
<h1>Environment Variables</h1>
<a href="https://nextjs.org/docs/basic-features/environment-variables">Read Docs</a>
<p>By default environment variables are only available in the Node.js environment, meaning they won&apos;t be exposed to the browser.</p>
<p>
<code>NEXT_PUBLIC_</code> environment variables are available in the browser, and can be used to configure the application.
By default environment variables are only available in the Node.js environment, meaning they won&apos;t be
exposed to the browser.
</p>
<p>
<code>NEXT_PUBLIC_</code> environment variables are available in the browser, and can be used to configure the
application.
</p>
<p>✅ Public Environment token found: <code>{process.env.NEXT_PUBLIC_GREETINGS || 'NOT FOUND (something went wrong)'}</code></p>
<p>❌ Private Environment token should not be found: <code>{process.env.TEST_ENV_VAR || 'Everything worked'}</code></p>
{showEnv ? (
<>
<p>
✅ Public Environment token found:{' '}
<code>{process.env.NEXT_PUBLIC_GREETINGS || 'NOT FOUND (something went wrong)'}</code>
</p>
<p>
❌ Private Environment token should not be found:{' '}
<code>{process.env.TEST_ENV_VAR || 'Everything worked'}</code>
</p>
</>
) : (
'We only want to test client-side env vars'
)}
</div>
)
}

export default EnvTest
export default EnvTest
18 changes: 6 additions & 12 deletions demos/default/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,17 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"target": "es5",
"lib": [
"dom",
"dom.iterable",
"esnext"
"es2019",
"dom"
],
"allowJs": true,
"skipLibCheck": true,
"strict": false,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"esModuleInterop": true,
"module": "esnext",
"incremental": true,
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve",
"incremental": true
"jsx": "preserve"
},
"include": [
"next-env.d.ts",
Expand All @@ -27,4 +21,4 @@
"exclude": [
"node_modules"
]
}
}
1 change: 0 additions & 1 deletion demos/next-export/local-plugin/index.js

This file was deleted.

1 change: 0 additions & 1 deletion demos/next-export/local-plugin/manifest.yml

This file was deleted.

5 changes: 0 additions & 5 deletions demos/next-export/local-plugin/package-lock.json

This file was deleted.

11 changes: 0 additions & 11 deletions demos/next-export/local-plugin/package.json

This file was deleted.

3 changes: 2 additions & 1 deletion demos/next-export/netlify.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF ../../"

[build.environment]
NETLIFY_NEXT_PLUGIN_SKIP = "true"
NODE_VERSION = "17"

[dev]
framework = "#static"

[[plugins]]
package = "./local-plugin"
package = "../plugin-wrapper/"

[[plugins]]
package = "@netlify/plugin-local-install-core"
22 changes: 22 additions & 0 deletions demos/next-export/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "next-export-demo",
"version": "1.0.0",
"description": "",
"devDependencies": {
"@netlify/plugin-nextjs": "*"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "git+https://github.com/netlify/netlify-plugin-nextjs.git"
},
"author": "",
"private": true,
"license": "MIT",
"bugs": {
"url": "https://github.com/netlify/netlify-plugin-nextjs/issues"
},
"homepage": "https://github.com/netlify/netlify-plugin-nextjs#readme"
}
2 changes: 1 addition & 1 deletion demos/nx-next-monorepo-demo/local-plugin/index.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = require('../../../lib')
module.exports = require('../../../plugin/lib')
2 changes: 2 additions & 0 deletions demos/plugin-wrapper/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// eslint-disable-next-line node/no-unpublished-require
module.exports = require('../../plugin/lib')
1 change: 1 addition & 0 deletions demos/plugin-wrapper/manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
name: 'plugin-wrapper'
15 changes: 15 additions & 0 deletions demos/plugin-wrapper/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "local-plugin",
"version": "1.0.0",
"main": "index.js",
"author": "",
"license": "MIT",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"devDependencies": {
"@netlify/plugin-nextjs": "*",
"husky": "^7.0.4",
"npm-run-all": "^4.1.5"
}
}
6 changes: 2 additions & 4 deletions demos/static-root/netlify.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@ ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF ../../"
# cache Cypress binary in local "node_modules" folder
# so Netlify caches it
CYPRESS_CACHE_FOLDER = "../node_modules/.CypressBinary"
NODE_VERSION = "17"

[dev]
framework = "#static"

[[plugins]]
package = "./local-plugin"
package = "../plugin-wrapper/"

[[plugins]]
package = "@netlify/plugin-local-install-core"

[[context.deploy-preview.plugins]]
package = "netlify-plugin-cypress"
Loading