webdriverio / webdriverio

Next-gen browser and mobile automation test framework for Node.js
http://webdriver.io
MIT License
9.1k stars 2.52k forks source link

[💡 Feature]: Check for unsupported `"engines"` package.json fields #11868

Open colinrotherham opened 11 months ago

colinrotherham commented 11 months ago

Is your feature request related to a problem?

Follow up to https://github.com/webdriverio/webdriverio/pull/11823#event-11239137852 creating it as an issue

With frequent Dependabot update PRs it's easy to merge updates that drop support for older Node.js versions

Can checks be added to prevent this?

Describe the solution you'd like.

For example, log output already includes silent warnings from npm ci when updates are incompatible:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@lerna/create@8.0.0',
npm WARN EBADENGINE   required: { node: '>=18.0.0' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'form-data-encoder@4.0.2',
npm WARN EBADENGINE   required: { node: '>= 18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'got@14.0.0',
npm WARN EBADENGINE   required: { node: '>=20' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'lerna@8.0.0',
npm WARN EBADENGINE   required: { node: '>=18.0.0' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }

Using npm engine-strict=true all incompatible updates from Dependabot PRs will fail to complete npm ci

Describe alternatives you've considered.

Using a package like installed-check

But it would need "workspaces" support as mentioned in https://github.com/webdriverio/webdriverio/pull/11823#issuecomment-1852075756

npx --workspaces installed-check --engine-check --engine-no-dev

It throws on node_modules not being in the same place as package.json

Unexpected error: Failed to list installed modules: Non-existing path set: /path/to/project/webdriverio/packages/devtools/node_modules

ErrorWithCause: Failed to list installed modules
   at file:///path/to/project/webdriverio/node_modules/installed-check-core/index.js:52:13

Additional context

It's not currently possible to use engine-strict=true with --omit=dev to check non-development dependencies

Note: Dependabot also uses its own Node.js version must be compatible with engine-strict=true. For example, using a minimum Node.js v20 "engines" field would prevent Dependabot running Node.js v18

Code of Conduct

wdio-bot commented 11 months ago

Thanks for reporting!

We greatly appreciate any contributions that help resolve the bug. While we understand that active contributors have their own priorities, we kindly request your assistance if you rely on this bug being fixed. We encourage you to take a look at our contribution guidelines or join our friendly Discord development server, where you can ask any questions you may have. Thank you for your support, and cheers!

voxpelli commented 8 months ago

I have released installed-check 9.0.0 which includes workspace support, possibly unblocking this

colinrotherham commented 7 months ago

@voxpelli Thanks for releasing the new update

I gave it try but the repo now uses pnpm so needs package.json "workspaces" back again

 WARN  The "workspaces" field in package.json is not supported by pnpm. Create a "pnpm-workspace.yaml" file instead.

But you can try it out on my branch https://github.com/webdriverio/webdriverio/compare/main...colinrotherham:node-engines with the following output:

Output

Errors:

e2e: svelte: Narrower "engines.node" is needed: >=16.0.0
e2e: @babel/plugin-transform-react-jsx-development: Narrower "engines.node" is needed: >=6.9.0
e2e: expect: Narrower "engines.node" is needed: ^14.15.0 || ^16.10.0 || >=18.0.0
e2e: mocha: Narrower "engines.node" is needed: >=14.0.0
e2e: puppeteer-core: Narrower "engines.node" is needed: >=16.0.0
e2e: string-width: Narrower "engines.node" is needed: >=8.0.0
e2e: vite: Narrower "engines.node" is needed: ^14.18.0 || >=16.0.0

Suggestions:

e2e: Combined "engines.node" needs to be narrower: ^16.10.0 || >=18.0.0
christian-bromann commented 7 months ago

@colinrotherham interesting, what would you suggest? It seems like there are some dependencies that have a wider engine field, e.g. include Node.js v14, and cause some issues here?

colinrotherham commented 7 months ago

See what @voxpelli thinks?

You're probably not as worried about packages such as string-width not having a maximum Node.js version

Other than ignoring packages, I'm not too sure whether the "Narrower… is needed" errors can be configured

voxpelli commented 7 months ago

Thanks for letting me know aboutpnpm-workspace.yaml, I'll add support for it :+1: https://github.com/voxpelli/read-workspaces/issues/2

I wouldn't say that its an issue that some packages support a wider version than you do – only if they have a narrower support range is it a trouble.

On a side note, you have to update this:

pnpm exec installed-check --engine-check --engine-no-dev

To this:

pnpm exec installed-check --engine-check --ignore-dev

As ignore-dev is also used by the new peer-check in 9.0.0 I decided to rename it and I didn't keep the old flag around

colinrotherham commented 7 months ago

Great spot, thanks @voxpelli I've updated https://github.com/webdriverio/webdriverio/compare/main...colinrotherham:node-engines

Adding pnpm support would be brilliant. Watch out for the dependency hoisting settings in .npmrc as hoist-workspace-packages will be enabled by default in v9 but can be configured either way still

Output

Errors:

e2e: svelte: Narrower "engines.node" is needed: >=16.0.0

Suggestions:

e2e: Combined "engines.node" needs to be narrower: >=16.0.0

Worth mentioning that my package.json workaround probably hasn't seen all package.json files

voxpelli commented 7 months ago

Released version 9.1.0 which has preliminary support for resolving pnpm workspaces + adds a new output in verbose mode that lists all successful and unsuccessful workspaces. Looks like this:

Skärmavbild 2024-04-04 kl  17 20 44

So if you run installed-check --engine-check --ignore-dev --verbose, then you will be able to see all workspaces it has found and whether it deemed them successful or not. This can be handy for initial setup / verification / debugging.

colinrotherham commented 7 months ago

Thanks @voxpelli 🙌

Output from --verbose was showing all the workspaces but we hit an issue following pnpm run setup

Each package now has a git-ignored ./build but is flagged as multiple workspaces with the same name

Output

Unexpected error: must not have multiple workspaces with the same name
package '@wdio/cli-cjs' has conflicts in the following paths:
    packages/wdio-cli/build/cjs
    packages/wdio-cli/src/cjs
package '@wdio/cucumber-framework-cjs' has conflicts in the following paths:
    packages/wdio-cucumber-framework/build/cjs
    packages/wdio-cucumber-framework/src/cjs
package '@wdio/json-reporter-cjs' has conflicts in the following paths:
    packages/wdio-json-reporter/build/cjs
    packages/wdio-json-reporter/src/cjs
package '@wdio/shared-store-service-cjs' has conflicts in the following paths:
    packages/wdio-shared-store-service/build/cjs
    packages/wdio-shared-store-service/src/cjs
package 'webdriver-cjs' has conflicts in the following paths:
    packages/webdriver/build/cjs
    packages/webdriver/src/cjs
package 'webdriverio-cjs' has conflicts in the following paths:
    packages/webdriverio/build/cjs
    packages/webdriverio/src/cjs

Error: must not have multiple workspaces with the same name
package '@wdio/cli-cjs' has conflicts in the following paths:
    packages/wdio-cli/build/cjs
    packages/wdio-cli/src/cjs
package '@wdio/cucumber-framework-cjs' has conflicts in the following paths:
    packages/wdio-cucumber-framework/build/cjs
    packages/wdio-cucumber-framework/src/cjs
package '@wdio/json-reporter-cjs' has conflicts in the following paths:
    packages/wdio-json-reporter/build/cjs
    packages/wdio-json-reporter/src/cjs
package '@wdio/shared-store-service-cjs' has conflicts in the following paths:
    packages/wdio-shared-store-service/build/cjs
    packages/wdio-shared-store-service/src/cjs
package 'webdriver-cjs' has conflicts in the following paths:
    packages/webdriver/build/cjs
    packages/webdriver/src/cjs
package 'webdriverio-cjs' has conflicts in the following paths:
    packages/webdriverio/build/cjs
    packages/webdriverio/src/cjs
    at getError (/path/to/project/webdriverio/node_modules/.pnpm/@npmcli+map-workspaces@3.0.4/node_modules/@npmcli/map-workspaces/lib/index.js:64:24)
    at mapWorkspaces (/path/to/project/webdriverio/node_modules/.pnpm/@npmcli+map-workspaces@3.0.4/node_modules/@npmcli/map-workspaces/lib/index.js:146:11)
    at async readWorkspaces (file:///path/to/project/webdriverio/node_modules/.pnpm/read-workspaces@1.1.1/node_modules/read-workspaces/index.js:80:15)
    at async workspaceLookup (file:///path/to/project/webdriverio/node_modules/.pnpm/list-installed@5.2.1/node_modules/list-installed/lib/lookup.js:47:20)
    at async installedCheck (file:///path/to/project/webdriverio/node_modules/.pnpm/installed-check-core@8.2.1/node_modules/installed-check-core/lib/installed-check.js:30:20)
    at async file:///path/to/project/webdriverio/node_modules/.pnpm/installed-check@9.1.0/node_modules/installed-check/cli.js:111:18
voxpelli commented 7 months ago

@colinrotherham It's because you use ** in pnpm-workspace.yaml rather than *, would it work do go with eg. packages/* instead of packages/**?

https://github.com/webdriverio/webdriverio/blob/a46cea223eadc22d3c68338dd58e20454c206917/pnpm-workspace.yaml#L2-L5

It probably occurs here because I bolted on the pnpm workspace resolving on top of the npm workspace resolving and only really used @pnpm/workspace.read-manifest from pnpm itself rather than the full on @pnpm/workspace.find-packages, as that latter one would do a lot of duplicated work: https://github.com/voxpelli/read-workspaces/blob/746cc1f0d92cdbbd81062b4f603c61bc9e619967/index.js#L135-L136

colinrotherham commented 7 months ago

I went ahead and tried narrowing those globs down. Deeply nested workspaces don't have dependencies

https://github.com/webdriverio/webdriverio/blob/a46cea223eadc22d3c68338dd58e20454c206917/pnpm-lock.yaml#L1400-L1402

But now it doesn't find the packages 😬

Presume this is because pnpm only symlinks workspace packages from node_modules/.pnpm/*?

Output

Warnings:

...more...

@wdio/browserstack-service: @wdio/cli: Dependency is not installed. Can't check its requirements
@wdio/cli: @types/node: Dependency is not installed. Can't check its requirements
@wdio/cli: @vitest/snapshot: Dependency is not installed. Can't check its requirements
@wdio/cli: @wdio/config: Dependency is not installed. Can't check its requirements
@wdio/cli: @wdio/globals: Dependency is not installed. Can't check its requirements
@wdio/cli: @wdio/logger: Dependency is not installed. Can't check its requirements
@wdio/cli: @wdio/protocols: Dependency is not installed. Can't check its requirements
@wdio/cli: @wdio/types: Dependency is not installed. Can't check its requirements
@wdio/cli: @wdio/utils: Dependency is not installed. Can't check its requirements
@wdio/cli: async-exit-hook: Dependency is not installed. Can't check its requirements
@wdio/cli: chalk: Dependency is not installed. Can't check its requirements
@wdio/cli: chokidar: Dependency is not installed. Can't check its requirements

...more...

Successful workspaces: eslint-plugin-wdio, @wdio/allure-reporter, @wdio/appium-service, @wdio/browser-runner, @wdio/browserstack-service, @wdio/cli, @wdio/concise-reporter, @wdio/config, @wdio/cucumber-framework, @wdio/devtools-service, @wdio/dot-reporter, @wdio/firefox-profile-service, @wdio/globals, @wdio/jasmine-framework, @wdio/json-reporter, @wdio/junit-reporter, @wdio/local-runner, @wdio/logger, @wdio/mocha-framework, @wdio/protocols, @wdio/repl, @wdio/reporter, @wdio/runner, @wdio/sauce-service, @wdio/shared-store-service, @wdio/smoke-test-cjs-service, @wdio/smoke-test-reporter, @wdio/smoke-test-service, @wdio/spec-reporter, @wdio/static-server-service, @wdio/sumologic-reporter, @wdio/testingbot-service, @wdio/types, @wdio/utils, @wdio/webdriver-mock-service, webdriver, webdriverio, bidi, cloudservices, devtools, multiremote, pageobject, standalone, wdio, mocha, vite-vue-example, website, e2e, smoke, root
voxpelli commented 7 months ago

I released a fix for symlinks 30 minutes ago, along with some other pnpm quality of life improvements: https://github.com/voxpelli/node-installed-check/releases/tag/v9.1.1

I will download your repository and do some more checking

voxpelli commented 7 months ago

After some digging I found https://github.com/pnpm/pnpm/issues/3398 and by adapting the suggestion in https://github.com/pnpm/pnpm/issues/3398#issuecomment-835277787 I could conclude that pnpm is also resolving the /build/ workspaces:

❯ pnpm ls -r --depth -1 --long --parseable | grep '@wdio/cli-cjs'
/Users/pelle/Sites/smallrepos/webdriverio/packages/wdio-cli/build/cjs:@wdio/cli-cjs:PRIVATE
/Users/pelle/Sites/smallrepos/webdriverio/packages/wdio-cli/src/cjs:@wdio/cli-cjs:PRIVATE

But unlike npm it doesn't seem like pnpm allows one to target a workspace by name (npm eg. allows for npm run -w @wdio/cli-cjs foo) and as such doesn't need the workspaces to be uniquely named, which is an unfortunate decision on their behalf as that makes them less compatible with npm.

I will have to ponder on a possible solution. I could probably expose a --workspace-ignore that I forward to the ignore in https://www.npmjs.com/package/@npmcli/map-workspaces and which you could then use like --workspace-ignore='**/build/**'

voxpelli commented 7 months ago

If you update to installed-check@9.2.0 then you can now do:

pnpm exec installed-check --engine-check --ignore-dev --workspace-ignore='**/build/**'

It makes the repo work after pnpm run setup. Thanks for helping me harden the tool 🙏

colinrotherham commented 7 months ago

That was quick, thanks!

I've updated https://github.com/webdriverio/webdriverio/compare/main...colinrotherham:node-engines

Is this output useful @christian-bromann?

Output

Errors:

@wdio/cucumber-framework: @cucumber/cucumber: Narrower "engines.node" is needed: ^18.0.0 || >=20.0.0
@wdio/json-reporter: @wdio/reporter: Narrower "engines.node" is needed: >=18.0.0
@wdio/json-reporter: @wdio/types: Narrower "engines.node" is needed: >=18.0.0
e2e: react: Narrower "engines.node" is needed: >=0.10.0
e2e: svelte: Narrower "engines.node" is needed: >=16.0.0
smoke: webdriver: Narrower "engines.node" is needed: >=18.0.0
smoke: webdriverio: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/cli: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/config: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/logger: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/repl: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/runner: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/utils: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/globals: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/allure-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/concise-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/dot-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/junit-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/spec-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/sumologic-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/appium-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/browserstack-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/devtools-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/firefox-profile-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/sauce-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/shared-store-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/testingbot-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/local-runner: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/browser-runner: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/cucumber-framework: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/jasmine-framework: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/mocha-framework: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/smoke-test-reporter: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/smoke-test-service: Narrower "engines.node" is needed: >=18.0.0
smoke: @wdio/webdriver-mock-service: Narrower "engines.node" is needed: >=18.0.0

Suggestions:

@wdio/cucumber-framework: Combined "engines.node" needs to be narrower: ^18.0.0 || >=20.0.0
@wdio/json-reporter: Combined "engines.node" needs to be narrower: >=18.0.0
e2e: Combined "engines.node" needs to be narrower: >=16.0.0
smoke: Combined "engines.node" needs to be narrower: >=18.0.0
voxpelli commented 7 months ago

I added a --fix in installed-check@9.3.0 as it makes more sense now with big monorepos like this. So you can now at least fix these easily 🙂

colinrotherham commented 7 months ago

Thanks @voxpelli, PR opened 👍