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.

Sign up for GitHub

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

Jump to bottom

Do not automate addition of engines field to all modules #1276

Closed
achingbrain opened this issue May 16, 2023 · 0 comments · Fixed by #1277
Closed

Do not automate addition of engines field to all modules #1276

achingbrain opened this issue May 16, 2023 · 0 comments · Fixed by #1277

Comments

@achingbrain
Copy link
Member

achingbrain commented May 16, 2023 •
edited
Loading

When the engines field is specified and the current node version doesn't meet it, npm shows a warning but yarn errors.

It's kept in sync with the aegir check-project command so gets updated occasionally.

In CI we only test on npm because we cannot support every permutation of every module installer. The intention behind the engines field is to be helpful to the user but when they use yarn it can end up breaking them unnecessarily.

Not all modules actually require the version set by check-project but it's too much work to go around and audit each individual module to work out which one requires what so I propose dropping the field from our semi-automated config updates.

If an individual module requires a certain minor release of node lts, it can add an engines field to it's package.json but we don't to need enforce that every module requires node 18+.

Refs: multiformats/js-mafmt#154 (comment)
Refs: multiformats/js-mafmt#152 (comment)

cc @SgtPooki @whizzzkid
@BigLep BigLep mentioned this issue May 16, 2023
Closed
achingbrain added a commit that referenced this issue May 17, 2023
@achingbrain
fix: do not enforce engines in package.json
40d70da 
The engines field causes warnings on npm but errors on yarn when the
required node version is not met.

Not all of our modules require LTS node and they are used by projects
outside of libp2p/helia/etc.

This change removes the automatic addition of an engines field to a
project's `package.json` while running the `check-project` command,
instead projects that do require a certain node version are free to
define it and deal with the breakage that subsequently occurs.

Partial revert of: #1184
Refs: #1276
@achingbrain achingbrain mentioned this issue May 17, 2023
Merged
achingbrain added a commit to multiformats/js-mafmt that referenced this issue May 17, 2023
@achingbrain
fix: remove engines field
9ce5c82 
When the current node version does not meet the engines field requirement npm warns but yarn errors.

Nothing in this module requires node 18+ so remove the engines field.

Refs: ipfs/aegir#1276
@achingbrain achingbrain mentioned this issue May 17, 2023
Merged
@achingbrain achingbrain changed the title Remove engines field from all modules Do not automate addition of engines field to all modules May 17, 2023
achingbrain added a commit to multiformats/js-mafmt that referenced this issue May 17, 2023
@achingbrain
fix: remove engines field (#155)
a464023 
When the current node version does not meet the engines field requirement npm warns but yarn errors.

Nothing in this module requires node 18+ so remove the engines field.

Refs: ipfs/aegir#1276
achingbrain added a commit that referenced this issue May 19, 2023
@achingbrain
fix: do not enforce engines in package.json (#1277)
5e9c2fa 
The engines field causes warnings on npm but errors on yarn when the
required node version is not met.

Not all of our modules require LTS node and they are used by projects
outside of libp2p/helia/etc.

This change removes the automatic addition of an engines field to a
project's `package.json` while running the `check-project` command,
instead projects that do require a certain node version are free to
define it and deal with the breakage that subsequently occurs.

Partial revert of: #1184
Refs: #1276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: do not enforce engines in package.json ipfs/aegir
1 participant
@achingbrain