-
Notifications
You must be signed in to change notification settings - Fork 80
Include engines
and devEngines
fields in package.json
?
#680
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
Comments
I'll take this one Mao, if you don't mind. Good call out! I forgot all about that. |
No problem at all, it'll likely be quicker that way. |
This is helpful - Heroku Node.js docs:
Going to wait to get access to the app before I PR the change, but it should be as simple as adding what the docs say. Emphasis on should. |
Awesome. Going by that then I'd assume Heroku updates their v22 minor automatically on/near release? |
Yep! As long as we don't peg it to an exact version. |
Wouldn't it be more appropriate to set a specific minor? Some features get added in minors so if it was set to |
@MaoShizhong the version pegging is more for Heroku, If I understand it correctly. If we aren't explicit, Heroku will update Node minor versions as they are released. I think I'd rather have bit more explicit control over what version Node the production bot is running. Does that make sense, or am I missing something? |
@JustWaveThings Hang on, are you saying you do want to set a specific minor version or do not want to do so (e.g. 22.X)? Your latest message implies the former while your original message implies the latter to me. I feel like we should set a specific minor so that it forces it to be developed in the same runtime version as in prod, else someone could run into errors if they run a lower minor version that's missing features from a later minor that the code is using. Example, Then again, the examples I could find of method additions like this are generally experimental and niche things, so depends how strict you want it to be. |
@MaoShizhong , I apologize for the confusion. I am with you. I think the best course of action is to set an explicit minor version of Node to avoid the issue you illustrated, as well as have more control over the production instance. |
Ah, I get what you meant by this now. I misread it as you suggesting we don't peg it to a specific minor |
Was looking more into this and came across (nvm-sh/nvm#651) where I learnt that npm can now read a "devEngines": {
"runtime": {
"name": "node",
"onFail": "error",
"version": "22.14"
}
}, will error if any npm command is run and it's not using Node 22.14.X, which is what I was originally requesting.
|
engines
field in package.json
?engines
and devEngines
fields in package.json
?
Uh oh!
There was an error while loading. Please reload this page.
The repo currently does not specify which version of Node the bot is deployed on, so there is a chance that code runs locally but breaks in prod if it's running on a version that doesn't have a newer feature.
Recently, I had a PR that Fred reviewed that broke with one commit because I unknowingly used a Node v22+ feature and he was still on v20. Finding out what version the bot is deployed on (if available, able to bump it to the latest host version?) then setting that as the max in
package.json
would help prevent any such bugs.Edit: Accidentally created a blank issue. Happy to add this in if we can found out the exact version the bot is deployed on.
The text was updated successfully, but these errors were encountered: