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: use pure ES modules at the monorepo top level #3970

Merged
merged 2 commits into from
Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 5 additions & 44 deletions .eslintrc.js β†’ .eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
'use strict'

const { overrides } = require('@netlify/eslint-config-node')

module.exports = {
extends: ['plugin:fp/recommended', '@netlify/eslint-config-node'],
parserOptions: {
sourceType: 'module',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ESLint configuration can be simplified in several ways, now that every package is using pure ES modules.

},
rules: {
strict: 2,

Expand Down Expand Up @@ -54,32 +55,17 @@ module.exports = {
],
},
],
'import/extensions': [2, 'ignorePackages'],
},
overrides: [
...overrides,
{
files: ['**/tests.{cjs,mjs,js}', '**/tests/**/*.{cjs,mjs,js}'],
rules: {
'node/no-missing-import': 0,
},
},
{
files: ['**/fixtures/**/*.{cjs,mjs,js}'],
rules: {
'import/no-unresolved': 0,
'node/no-missing-import': 0,
},
},
{
files: ['**/fixtures/**/*edge-handlers*/**/*.js', '**/fixtures/*es_module*/**/*.js'],
parserOptions: {
sourceType: 'module',
},
rules: {
'node/no-unsupported-features/es-syntax': 0,
'ava/no-import-test-files': 0,
},
},

// @todo As it stands, this rule is problematic with methods that get+send
// many parameters, such as `runStep` in `src/steps/run_step.js`.
// We should discuss whether we want to keep this rule or discontinue it.
Expand Down Expand Up @@ -123,30 +109,5 @@ module.exports = {
'import/no-anonymous-default-export': 0,
},
},

{
files: ['packages/*/tests/**/*.{cjs,mjs,js}'],
rules: {
'no-magic-numbers': 'off',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-enable this rule since only a few statements were offending.

},
},

// Those packages are using pure ES modules
{
files: [
'packages/build/**/*.{cjs,mjs,js}',
'packages/cache-utils/**/*.{cjs,mjs,js}',
'packages/config/**/*.{cjs,mjs,js}',
'packages/functions-utils/**/*.{cjs,mjs,js}',
'packages/git-utils/**/*.{cjs,mjs,js}',
'packages/run-utils/**/*.{cjs,mjs,js}',
],
parserOptions: {
sourceType: 'module',
},
rules: {
'import/extensions': [2, 'ignorePackages'],
},
},
],
}
2 changes: 0 additions & 2 deletions commitlint.config.js β†’ commitlint.config.cjs
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
'use strict'

module.exports = { extends: ['@commitlint/config-conventional'] }
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"private": true,
"version": "0.0.0",
"description": "Netlify build module",
"main": "index.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.

The top-level package.json is not published to npm and does not have any main/exports file.

"type": "module",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the main change.

"author": "Netlify Inc.",
"scripts": {
"test": "run-s format test:dev",
Expand Down
6 changes: 4 additions & 2 deletions packages/build/tests/core/tests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ const getNodeBinary = async function (nodeVersion, retries = 1) {
const MAX_RETRIES = 10

// Memoize `get-node`
const mGetNode = moize(getNodeBinary, { isPromise: true, maxSize: 1e3 })
const GET_NODE_MOIZE_MAX_SIZE = 1e3
const mGetNode = moize(getNodeBinary, { isPromise: true, maxSize: GET_NODE_MOIZE_MAX_SIZE })

test('--node-path is used by build.command', async (t) => {
const { path } = await mGetNode(CHILD_NODE_VERSION)
Expand Down Expand Up @@ -357,7 +358,7 @@ test.serial('Passes the right base path properties to zip-it-and-ship-it', async
t.is(repositoryRoot, fixtureDir)
})

// eslint-disable-next-line max-statements
/* eslint-disable max-statements, no-magic-numbers */
test.serial('Passes the right feature flags to zip-it-and-ship-it', async (t) => {
// eslint-disable-next-line import/no-named-as-default-member
const mockZipFunctions = sinon.stub().resolves()
Expand Down Expand Up @@ -417,6 +418,7 @@ test.serial('Passes the right feature flags to zip-it-and-ship-it', async (t) =>
t.true(mockZipFunctions.getCall(7).args[2].featureFlags.this_is_a_mock_flag)
t.true(mockZipFunctions.getCall(7).args[2].featureFlags.and_another_one)
})
/* eslint-enable max-statements, no-magic-numbers */

test('Print warning on lingering processes', async (t) => {
const { returnValue } = await runFixture(t, 'lingering', {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import isPlainObj from 'is-plain-obj'

// eslint-disable-next-line ava/no-import-test-files
import data from './data.json'

export const onRequest = function (event) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[[plugins]]
package = "./plugin"
package = "./plugin.cjs"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some test files must be renamed to .cjs to get the right linting.

Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[[plugins]]
package = "./plugin"
package = "./plugin.cjs"
16 changes: 8 additions & 8 deletions packages/build/tests/plugins/snapshots/tests.mjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,21 @@ Generated by [AVA](https://avajs.dev).
plugins:␊
- inputs: {}␊
origin: config␊
package: ./plugin␊
package: ./plugin.cjs␊
␊
> Context␊
production␊
␊
> Loading plugins␊
- ./plugin@1.0.0 from netlify.toml␊
- ./plugin.cjs@1.0.0 from netlify.toml␊
␊
β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€βŠ
1. ./plugin (onPreBuild event) ␊
1. ./plugin.cjs (onPreBuild event) ␊
β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€βŠ
␊
true␊
␊
(./plugin onPreBuild completed in 1ms)␊
(./plugin.cjs onPreBuild completed in 1ms)␊
␊
β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€βŠ
Netlify Build Complete ␊
Expand Down Expand Up @@ -2564,21 +2564,21 @@ Generated by [AVA](https://avajs.dev).
plugins:␊
- inputs: {}␊
origin: config␊
package: ./plugin␊
package: ./plugin.cjs␊
␊
> Context␊
production␊
␊
> Loading plugins␊
- ./plugin@1.0.0 from netlify.toml␊
- ./plugin.cjs@1.0.0 from netlify.toml␊
␊
β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€βŠ
1. ./plugin (onPreBuild event) ␊
1. ./plugin.cjs (onPreBuild event) ␊
β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€βŠ
␊
JavaScript␊
␊
(./plugin onPreBuild completed in 1ms)␊
(./plugin.cjs onPreBuild completed in 1ms)␊
␊
β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€βŠ
Netlify Build Complete ␊
Expand Down
Binary file modified packages/build/tests/plugins/snapshots/tests.mjs.snap
Binary file not shown.
4 changes: 3 additions & 1 deletion packages/build/tests/telemetry/tests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,11 @@ test('Telemetry calls timeout by default', async (t) => {
// We want to rely on the default timeout value
disableTelemetryTimeout: false,
// Introduce an arbitrary large timeout on the server side so that we can validate the client timeout works
waitTelemetryServer: 5 * 60 * 1000,
waitTelemetryServer: WAIT_TELEMETRY_SERVER,
// The error monitor snapshot should contain the timeout error
snapshot: true,
})
t.is(telemetryRequests.length, 0)
})

const WAIT_TELEMETRY_SERVER = 3e5