Wrong notation in package.json

[Crashillo]

🎩 What? Why?

According to https://docs.npmjs.com/cli/v8/configuring-npm/package-json#engines the wrong notation creates output warnings

📌 Related Issues

#8343

Testing

Just run the app

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

• ❓ CONSIDER adding a unit test if your PR resolves an issue.
• ✔️ DO check open PR's to avoid duplicates.
• ✔️ DO keep pull requests small so they can be easily reviewed.
• ✔️ DO build locally before pushing.
• ✔️ DO make sure tests pass.
• ✔️ DO make sure any new changes are documented in docs/.
• ✔️ DO add and modify seeds if necessary.
• ✔️ DO add CHANGELOG upgrade notes if required.
• ✔️ DO add to GraphQL API if there are new public fields.
• ✔️ DO add link to MetaDecidim if it's a new feature.
• ❌AVOID breaking the continuous integration build.
• ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

None

♥️ Thank you!

[andreslucena]

@Crashillo @leio10 isn't working with Decidim anymore

According to docs.npmjs.com/cli/v8/configuring-npm/package-json#engines the wrong notation creates output warnings

Do you mean this section?

Unless the user has set the engine-strict config flag, this field is advisory only and will only produce warnings when your package is installed as a dependency.

What do you mean with output warnings? Have you seen one of those in Decidim (we probably have, but just to document better exactly what we're trying to solve).

Please check the failing specs, you need to change the others package.json too.

[Crashillo]

What do you mean with output warnings?

Running for instance, bin/rails decidim:webpacker:install

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@decidim/all@0.25.0-dev',
npm WARN EBADENGINE   required: { node: '^15.14.0', npm: '^7.7.2' },
npm WARN EBADENGINE   current: { node: 'v16.13.0', npm: '8.1.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@decidim/core@0.26.0-dev',
npm WARN EBADENGINE   required: { node: '^16.9.1', npm: '^7.21.1' },
npm WARN EBADENGINE   current: { node: 'v16.13.0', npm: '8.1.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@decidim/all@0.25.0-dev',
npm WARN EBADENGINE   required: { node: '^15.14.0', npm: '^7.7.2' },
npm WARN EBADENGINE   current: { node: 'v16.13.0', npm: '8.1.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@decidim/core@0.26.0-dev',
npm WARN EBADENGINE   required: { node: '^16.9.1', npm: '^7.21.1' },
npm WARN EBADENGINE   current: { node: 'v16.13.0', npm: '8.1.0' }
npm WARN EBADENGINE }

Decidim is able to run with any node v16 upward, hence the engine property should be specified with >= symbols, better than ^. IMO, tt creates unnecessary noise.

Please check the failing specs, you need to change the others package.json too.

Yupp, I missed packages/core/package.json. I'm wondering if it's actually neccessary to have the engines property there, since the top-level package.json is already flagged like that. Is it there for any reason? Wouldn't be better to get rid of the inner condition?

[andreslucena]

Is it there for any reason? Wouldn't be better to get rid of the inner condition?

Skimming through the last PR that changes those lines (#8343), it seems like this is the only package.json from packages/ that has the engines key. As I'm preparing the release of v0.26, I'll merge this one after I have the new branch, so we can test locally at least in develop (for v0.27.0)