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

#330 Upgrade CCM To ESM #433

Conversation

pushyamig
Copy link
Contributor

@pushyamig pushyamig commented Apr 12, 2024

Fixes #330
Upgrade CCM as ESM Project. Not All package that CCM uses ESM so It's a mix bag of CJM and ESM. Node current version know how to load CJS in ESM package. The package developers also needs to support this interoperability. Some package like p-limit is purely ESM so won't work if CCM was CJS.

Most of the key changes are associated with configuration file like package.json, webpack, tsconfig.json, start.sh

Latest PR Build https://github.com/pushyamig/canvas-course-manager-next/actions and is being deployed to CCM Dev

  1. This is a good read on what is ESM Upgrade looks Like
  2. ts-node support for ESM PR is still WIP
  3. Update p-limit to v5 #430 Staying with V4
  4. Plan for removing GOT library from CCM build #431 Staying on same version as @kth/canvas-api
  5. As part of ESM, we need to have file extension .js when importing a file of CCM. So the all the files are touched
  6. Certain MUI file import needs some changes
  7. ESLint is currently with v8.57.0 since @typescript-eslint/eslint-plugin and @typescript-eslint/parser depends on older version if I upgrade to latest.
    1. ESLINT V9 has some breaking change so we can hold on to this version for later releases
    2. Liniting check is only part of VScode and Codacy based on the .eslintrc.js. So we can just upgrade in next release
  8. Test Plan is added to the issue Observe and plan for ecosystem move to ECMAScript Modules #330

@pushyamig pushyamig marked this pull request as ready for review April 12, 2024 18:24
@pushyamig pushyamig changed the title I330 alt cjs esm upgrade #330 Upgrade CCM To ESM Apr 12, 2024
@jaydonkrooss
Copy link
Contributor

So just so I understand, you're saying we can't upgrade p-limit because it isn't interoperable with CJS and ts-node isn't properly compiling it, in development only. So we're just waiting for ts-node to support the "sub-path import" feature before p-limit upgrade?

@pushyamig
Copy link
Contributor Author

pushyamig commented Apr 15, 2024

So just so I understand, you're saying we can't upgrade p-limit because it isn't interoperable with CJS and ts-node isn't properly compiling it, in development only. So we're just waiting for ts-node to support the "sub-path import" feature before p-limit upgrade?

Yes, Right now that's my understanding. I feel we don't need to rush p-limit to upgrade v5 we will see how that PR with ts-node flows, otherwise we might have to come up with some hacky way to make it work which I felt can hold on for time being. Currently it is upgraded from V3 -> v4 which is still an ESM module.

I felt a bit waxed/fatigued with this particular issue so we can come back to it later

@pushyamig
Copy link
Contributor Author

p-limt v5 build

This is the Build/run that was successful when running Prod mode: https://github.com/pushyamig/canvas-course-manager-next/actions/runs/8651060404/job/23720922489

But Locally the Build/or Run will have errors with V5. SO rolled back https://github.com/pushyamig/canvas-course-manager-next/actions/runs/8652300727

@jaydonkrooss
Copy link
Contributor

Went through the test plan, locally testing features and migrations; all looks like it behaves as expected to me. Code looks good too.

@pushyamig pushyamig merged commit 137ce0f into tl-its-umich-edu:2024-03-01-dep-update Apr 16, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants