build: bump compatibility versions of the iOS SDK and Node.js #13948
Conversation
|
It is the correct place. The CLI will check for |
|
My Linux tests with node 18: |
|
It seems like Node.js 18 is in maintenance mode already since two weeks, so I am checking if we can go to 20 directly. iOS and Android app builds + module build work fine so far. |
| @@ -166,7 +166,7 @@ | |||
| "url": "git://github.com/tidev/titanium_mobile.git" | |||
| }, | |||
| "vendorDependencies": { | |||
| "node": "12.x || 14.x || 16.x" | |||
| "node": "12.x || 14.x || 16.x || 18.x || 20.x" | |||
There was a problem hiding this comment.
@cb1kenobi I wonder if we should remove everything pre-16, but in a SDK 13.0.0
There was a problem hiding this comment.
Here's what I propose:
- Don't touch anything in CLI 6.x or SDK 12.x
- In CLI v7 (Titanium CLI v7 Beta 1 titanium-cli#618), min Node version set to 18 (done)
- In CLI v7, rip out the
vendorDependenciescheck (todo) - In SDK 13, do nothing
- In SDK 14, do nothing
- In SDK 15, rip out vendor dependencies
Why rip out vendor dependencies?
- It's a pain to maintain
- "engines" didn't exist back in the day, but now it's a tool we can use to keep users moving forward
- Node support dictated by actively supported SDK releases, so regular release cadence will drop support for older Node.js versions naturally
- Rarely do we have Node compatibility issues with SDK dependencies
Note that we cannot remove vendorDependencies from the SDK's package.json until every user has updated to CLI v7. CLI v6 explicitly checks for that property with no guard. :(
There was a problem hiding this comment.
Please, please, please do not rip out anything or break things for now. As nice as the CLI v7 is, it's far away from a stable / tested release and will require some major upgrades to run stable. Fixing the warning in SDK 12.x should be the most convenient way for most users to continue working without updating their Node.js setup, while developers that have the latest LTS tooling can continue working as well, but without warnings.
There was a problem hiding this comment.
+1 for removing it (vendorDependencies)!
But I think we should add up to 20 in the package.json for 12.x just to get rid of the warning as there are no issues with using it - keeping the minimum as it is for now.
This makes it easier for new users to just grab the latest node version from the page and not have a warning right away
There was a problem hiding this comment.
Sorry, yes, add 18 and 20 to SDK 12 to suppress the warning.
Definitely not suggesting we break anything anytime soon. Once v7 is merged and published as latest (it'll be a while), we can circle back and do a final v6 release that adds a guard around vendorDependencies and a message to update to v7. I want to avoid touching the CLI until v6 is in a maintenance branch and v7 is merged into master. The merge conflicts are going to be impossible to fix.
|
My Linux tests with node 20: works fine too 👍 |
|
Not accepted. |
Note: I am not sure if this change will remove the Node.js warning or if the log originates from somewhere else: