search

microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
924 stars 287 forks source link

Update Node to LTS version #1767

Closed gavinbarron closed 1 year ago

gavinbarron commented 1 year ago

Proposal: Update development environments to use Node v16 or v18

Description

Currently we require developers to use Node v12 or v14 https://github.com/microsoftgraph/microsoft-graph-toolkit/wiki#Setup-the-development-environment The current LTS version is v16, this is also required for working with spfx https://docs.microsoft.com/en-us/sharepoint/dev/spfx/set-up-your-development-environment

Rationale

Neither of the Node versions that we advise developer to use are current versions, and given the current crossover between SharePoint development and where we see usage of MTG we should take steps to ensure that it's as easy as possible for a SharePoint developer to contribute back to our repository.

Preferred Solution

Ensure that a developer can build the source code for MTG using either Node 16 or 18. Have CI guardrails to ensure that solutions using MTG as a dependency will build running on any of the even numbered Node version that are active, current, or maintained (as of now this is 14, 16, and 18)

Related work items

ghost commented 1 year ago

Hello gavinbarron, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

gavinbarron commented 1 year ago

Initial blockers: samples/sp-webpart has dependency on @microsoft/sp-core-library@1.13.0 which locks the node version < 15.5.0

Update sample to use spfx@1.15

node-sass must be > 6.0 to run on node 16+ samples/react-app uses react-scripts@3.0.0 this introduces a transitive dependency on jest@24.9.0 which fails to meet the requirement for jest@">=26 <27" from ts-jest@26.5.6

The dependency on ts-jest appears to be outdated as the last tests using Jest in the library were removed in #767

musale commented 1 year ago

Hello @gavinbarron the latest CI run on main fails due to an incompatible node version 00865f30255e7b86c2df369e98bf9059c4601beb, is this a proper time to do the CI updates for the allowed node versions in the matrix?

gavinbarron commented 1 year ago

@musale yes, I think that we should be updating our supported node versions to 14 and 16 as part of the v3 release.

I suspect that we'll have quite a few dependencies which might cause us issues as we do so but now is the time.

musale commented 1 year ago

@gavinbarron true. Currently, I locked responselike to v2.0.0 to skip the build errors we had in main. As I was doing that, I managed to get v14.x of node to run builds. However, in azure devops it was failing because the node version was locked to v12.x. So in our updates, we should also keep that in mind. v16.x had a lot of dependencies that needed to be changed, as well as APIs.

gavinbarron commented 1 year ago

I saw that resolutions update @musale, thanks for that. How about we update the matrix we're using to validate builds to cover 12,14, and 16 on our next branch so that we can gauge our progress on this? We'd likely need to accept failures on 14 and 16 as we progress on this work. We could even use an additional action for this validation. Thoughts?

musale commented 1 year ago

Yes we can! That would also be a good point to start before moving it to the main branch. I think with v16.x we shall have the most work.