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.

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

Node 16 Upgrade: Initial survey and determine plan of action #2

Closed
24 of 37 tasks
davidjoy opened this issue Aug 24, 2021 · 10 comments
Closed
24 of 37 tasks

Node 16 Upgrade: Initial survey and determine plan of action #2

davidjoy opened this issue Aug 24, 2021 · 10 comments
Assignees

Comments

@davidjoy
Copy link
Contributor

davidjoy commented Aug 24, 2021

Prior to the Maple release being cut on October 9, we need to upgrade node to v16.

Note, Prospectus and the edX website team are considering being guinea pigs and starting to upgrade to node 16 themselves. This is a good thing.

A task describing the upgrade from Node 8 to 12: https://openedx.atlassian.net/browse/ARCH-352

How do we find versions we need to upgrade?

Tech ownership spreadsheet?

Action Items

  • @davidjoy Go talk to Diana. She ran this upgrade the last time from 8 to 12 and probably knows what we need to do.
  • @binodpant Enterprise Information gathering (add sections here for different areas of the codebase, dump information, get us the complete picture)
  • @muselesscreator Content information gathering (find places we're using node 12)
  • @davidjoy Engagement information gathering
  • @davidjoy Open edX information gathering
  • @adamstankiewicz Upgrade npm from 6.x to 7.x? Is this related to the node upgrade or can they be separated?
  • @davidjoy edx-platform and other legacy IDAs
  • Note that node-sass less than 6.x does not support node 16. We use node-sass 5.x in Paragon. See some details here: https://stackoverflow.com/questions/67241196/error-no-template-named-remove-cv-t-in-namespace-std-did-you-mean-remove
  • @binodpant determine what needs to be done next to merge build: upgrade node to v16.12.0, suggested npm is v8.1.0 frontend-app-profile#491
  • @binodpant determine/do if frontend-platform can be upgrade on its own to use new node, without impacting current workflows of devs/MFEs
  • @binodpant determine what changes are needed in frontend-build and frontend-platform to successfully build a MFE that depends on it, using npm upgrade
  • finish investigation/changes around enterprise MFE peer deps failing with upgrade attempt
  • what steps if any the owners of the various MFE repos need to take (within edX)
  • communication around getting ready with an install of nvm on devs' systems to be sent out
  • what steps should be published for external committers
  • where/how to document the steps devs may need to take post the upgrade

Things to check for each repo/project

  • Github actions workflows: will be automated by ArbiBOM tooling will create PRs for upgrades
  • [x ] travis.yml
  • Tubular build scripts? ArbiBOM will handle
  • .nvmrc added to repos: NOT NEEDED ANYMORE! ArbiBom will handle this via modernizr
  • Check package.json for evidence of node versions ("engines")
  • Write up a clear set of instructions and options for getting the work done (You do it, we do it, BD does it, etc.)
  • Communication: get engineers to update their dev environments
  • Docker (docker-compose.yml, etc.)
  • AMIs? (Amazon machine images, may manifest in Terraform)
  • prospectus (website is upgrading it)

Enterprise

By: @binodpant

Terraform check: I don't see the word npm in edx-terraform, so ...

edx-enterprise

learner portal

admin portal

Uses hardcoded node_version 12

  • engines in package.json
  • nvmrc usage in ci.yml? ( Uses hardcoded node_version 12)

public catalog

https://github.com/edx/frontend-app-enterprise-public-catalog/blob/main/.github/workflows/ci.yml
Uses hardcoded node_version 12

  • engines in package.json
  • nvmrc usage in ci.yml? ( Uses hardcoded node_version 12)

frontend-enterprise

@adamstankiewicz
Copy link
Member

adamstankiewicz commented Sep 14, 2021

Notes from meeting on 9/14:

@adamstankiewicz @binodpant @muselesscreator

  • We're going to keep NPM v7 upgrade alongside Node 16 upgrade, but we need to figure out where peer dependencies need to be updated to have semver ranges. @adamstankiewicz to take this on as an action item. We will prioritize this work.
  • @binodpant continue investigation into package.json engines to determine if they're needed, or what they actually do (e.g., throw warnings).
  • Binod also added some stuff related to Enterprise above.
  • Using nvm locally should be enforced to ensure consistency. Package.json engines may be helpful here. Documentation might help too in READMEs or PR templates (e.g., frontend-build, frontend-platform), but because it's for each frontend related repo, it's tricky... Could this be part of fedx-scripts in frontend-build to enforce versions while trying to execute commands such as start, test, etc.? Do you have ``.nvmrcfile locally andnvm` installed? Commands work if you have everything necessary (@adamstankiewicz)
  • Binod asks are there any conflicts between React 17 and Node 16 upgrades? TBD. @binodpant

Action items

  • @adamstankiewicz Document where we have peer dependency issues in known shared libraries.
  • @binodpant Continue investigation into what the engines field in package.json files are for; can they help enforce Node/NPM versions?
    • in my opinion we can use engines so that we can try and standardize versions and avoid issues. Per engines doc

    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

  • @adamstankiewicz Investigate how we might be able to enforce nvm usage and Node/NPM versions via fedx-scripts in @edx/frontend-build.

@davidjoy davidjoy removed their assignment Oct 4, 2021
@adamstankiewicz
Copy link
Member

We may also want to look into using node-alpine to reduce image size. Some more details here: https://blog.tarkalabs.com/how-to-reduce-nodejs-docker-image-by-70-e799b3d3396a

@binodpant
Copy link

binodpant commented Oct 26, 2021

As discussed last week I am going to start by upgrading frontend-app-profile. Based on learning from this, I will proceed to upgrading one library

Frontend-app-profile builds and tests successfully locally

nvm install v16.12.0 installed node 16.12.0 and npm 8.1.0 for me

I also added an .nvmrc file

Also, looks like we may be able to use this tool to use desired .nvmrc version of node/npm in GH: https://github.com/dcodeIO/setup-node-nvm

Node v16.12.0 is the latest v16 version of node, so we can go with that.
node release notes: https://github.com/nodejs/node/releases/tag/v16.12.0

~/work/frontend-app-profile (bpant/npm16-upgrade ✘)✭ ᐅ npm -v
8.1.0
~/work/frontend-app-profile (bpant/npm16-upgrade ✘)✭ ᐅ node -v
v16.12.0

@binodpant
Copy link

One thing I noticed in running npm install, even though it did not seem to fail the build, was this, on first time load

~/work/frontend-app-profile (bpant/npm16-upgrade ✘)✭ ᐅ npm install
npm WARN old lockfile
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile
npm WARN old lockfile HttpErrorGeneral: 404 Not Found - GET https://registry.npmjs.org/@edx%2fbrand - Not found
npm WARN old lockfile     at /Users/binodpant/.nvm/versions/node/v16.12.0/lib/node_modules/npm/node_modules/npm-registry-fetch/check-response.js:95:15
npm WARN old lockfile     at runMicrotasks (<anonymous>)
npm WARN old lockfile     at processTicksAndRejections (node:internal/process/task_queues:96:5)
npm WARN old lockfile     at async Array.<anonymous> (/Users/binodpant/.nvm/versions/node/v16.12.0/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:741:9)
npm WARN old lockfile  Could not fetch metadata for @edx/brand@1.1.0 HttpErrorGeneral: 404 Not Found - GET https://registry.npmjs.org/@edx%2fbrand - Not found
npm WARN old lockfile     at /Users/binodpant/.nvm/versions/node/v16.12.0/lib/node_modules/npm/node_modules/npm-registry-fetch/check-response.js:95:15
npm WARN old lockfile     at runMicrotasks (<anonymous>)
npm WARN old lockfile     at processTicksAndRejections (node:internal/process/task_queues:96:5)
npm WARN old lockfile     at async Array.<anonymous> (/Users/binodpant/.nvm/versions/node/v16.12.0/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:741:9) {
npm WARN old lockfile   headers: [Object: null prototype] {
npm WARN old lockfile     date: [ 'Tue, 26 Oct 2021 13:12:27 GMT' ],
npm WARN old lockfile     'content-type': [ 'application/json' ],
npm WARN old lockfile     'content-length': [ '21' ],
npm WARN old lockfile     connection: [ 'keep-alive' ],
npm WARN old lockfile     'expect-ct': [
npm WARN old lockfile       'max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"'
npm WARN old lockfile     ],
npm WARN old lockfile     vary: [ 'Accept-Encoding' ],
npm WARN old lockfile     server: [ 'cloudflare' ],
npm WARN old lockfile     'cf-ray': [ '6a43efb3696305a8-IAD' ],
npm WARN old lockfile     'x-fetch-attempts': [ '1' ],
npm WARN old lockfile     'x-local-cache-status': [ 'skip' ]
npm WARN old lockfile   },
npm WARN old lockfile   statusCode: 404,
npm WARN old lockfile   code: 'E404',
npm WARN old lockfile   method: 'GET',
npm WARN old lockfile   uri: 'https://registry.npmjs.org/@edx%2fbrand',
npm WARN old lockfile   body: { error: 'Not found' },
npm WARN old lockfile   pkgid: '@edx/brand@1.1.0'
npm WARN old lockfile }

@binodpant
Copy link

Further details will be in this PR for frontend-app-profile: openedx/frontend-app-profile#491

@binodpant
Copy link

I find, sort of as expected, that when trying to upgrade node in an Enterprise repo like Enterprise learner portal we get an error like this

~/work/frontend-app-learner-portal-enterprise (master ✘)✖✹ ᐅ npm i  
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: frontend-app-learner-portal-enterprise@0.1.0
npm ERR! Found: @edx/paragon@16.14.7
npm ERR! node_modules/@edx/paragon
npm ERR!   @edx/paragon@"16.14.7" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer @edx/paragon@">=10.0.0 <15.0.0" from @edx/frontend-platform@1.11.0
npm ERR! node_modules/@edx/frontend-platform
npm ERR!   @edx/frontend-platform@"1.11.0" from the root project
npm ERR!   peer @edx/frontend-platform@"^1.8.0" from @edx/frontend-component-footer@10.1.0
npm ERR!   node_modules/@edx/frontend-component-footer
npm ERR!     @edx/frontend-component-footer@"10.1.0" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 
npm ERR! See /Users/binodpant/.npm/eresolve-report.txt for a full report.

so yes this reveals that for projects that use frontend-platform, we will need to some legwork on the platform repo to help address this

this may serve as a starting point for working on some of the repoes, upgrade-wise

@davidjoy
Copy link
Contributor Author

davidjoy commented Nov 1, 2021

To me this means we're now forced to make sure our peer dependencies are "correct". Annoying, but probably healthy. :)

@binodpant
Copy link

binodpant commented Nov 9, 2021

Quick status:

Could not spend much time this week on this, but here is the latest:

  • I think I got everything needed for an npm/node upgrade for the frontend-app-profile repo, so that should be set to merge once approved
  • Next steps:
    • determine what needs to be done next to merge build: upgrade node to v16.12.0, suggested npm is v8.1.0 frontend-app-profile#491
    • determine/do if frontend-platform can be upgrade on its own to use new node, without impacting current workflows of devs/MFEs
    • determine what changes are needed in frontend-build and frontend-platform to successfully build a MFE that depends on it, using npm upgrade
    • finish investigation/changes around enterprise MFE peer deps failing with upgrade attempt
    • what steps if any the owners of the various MFE repos need to take (within edX)
    • communication around getting ready with an install of nvm on devs' systems to be sent out
    • what steps should be published for external committers
    • where/how to document the steps devs may need to take post the upgrade

@davidjoy
Copy link
Contributor Author

davidjoy commented Dec 8, 2021

Some useful things I just discovered:

  1. nvm suggests some code you can add to your shell's profile to get automatic node/npm version switching. Find your shell here: https://github.com/nvm-sh/nvm#deeper-shell-integration. I tried the zsh version just now, and it worked great.
  2. If you put an .nvmrc in your workspace directory above all your repos and put 12 in it, you'll switch back to node 12/npm 6 for all your frontend repos that don't yet have an .nvmrc. I kept forgetting to switch back when I left frontend-platform.

@davidjoy davidjoy removed this from the Maple milestone Feb 10, 2022
@binodpant binodpant added this to the node 16 upgrade milestone Feb 17, 2022
@binodpant binodpant changed the title Node 16 Upgrade Node 16 Upgrade: Initial survey and determine plan of action Feb 17, 2022
@binodpant
Copy link

Closing this task per discussion with team last week.

The idea is that we have done the preliminary investigation into how we will proceed.

A new set of issues will be created under the Milestone 'node 16 upgrade' to execute specific tasks. But this task can be considered closed now.

Here is a quick summary of how we will proceed:

Document laying out the process that will be followed:

https://openedx.atlassian.net/wiki/spaces/AC/pages/3318054984/Node+16+Upgrade

Feedback on process/improvements, will be taken/discussed in the document and/or in the weekly meetup for the FWG.

See the milestone for further updates on this project: https://github.com/openedx/frontend-wg/milestone/9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants