web icon indicating copy to clipboard operation
web copied to clipboard

feat: move from CJS to ESM

Open 43081j opened this issue 1 year ago • 8 comments

this is an unreasonably large change which does a few things:

  • Moves all packages to type: "module"
  • Removes node-fetch (ran into problems in ESM-land)
  • Replaces all __dirname with an equivalent using import.meta
  • Replaces all require.resolve with import.meta.resolve
  • Update all imports to be ESM-compatible
  • Move all tsconfig to use nodenext resolution

Probably some other stuff too.

@koddsson if your branch is cleaner, and easier to review etc etc just let me know. im happy to throw this away, it was still a learning experience and i did go upstream to fix a few dependencies we have to make them work in nodenext.

ah yeah and it'll be outdated, probably a truckload of conflicts but i can update if we want it

43081j avatar Nov 21 '23 19:11 43081j

⚠️ No Changeset found

Latest commit: aa809a731cb912dfed0fd715e3d3e793bd25b125

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Nov 21 '23 19:11 changeset-bot[bot]

I think a good way forward is to create a next branch and push my PR to that branch and make a prerelease. Then we can rebase this PR against the next branch which makes it easier to look at the diff of your changes and my changes and get this PR running green and then merge this and possible other PRs to next and keep releasing prereleases until we're happy and do a actual major release.

koddsson avatar Nov 22 '23 07:11 koddsson

Actually I forgot I just commented out some tests in that PR. I gotta get those fixed first.

koddsson avatar Nov 22 '23 08:11 koddsson

Actually I forgot I just commented out some tests in that PR. I gotta get those fixed first.

Ended up fixing this. I've pushed my PR to the next branch where the action has published all those packages as canary packages to npm so we can start testing those out.

If you rebase this PR against next we can look at the diff and start pulling relevant parts into the next branch and therefore the canary npm tag.

koddsson avatar Nov 22 '23 09:11 koddsson

i haven't had chance to look yet what you've done. it sounds like you've done a lot of what i've done here, but the shorter route (e.g. createRequire).

i went all in on this branch instead so its probably less likely to land. for example:

  • all fake paths have been removed (the aliases in export maps which don't actually exist, i.e. you now have to import @web/whatever/dist/whatever.js rather than @web/whatever/whatever.js)
  • all CJS support has been dropped (from export maps, builds, etc)
  • upgraded a few dependencies to make them happy in nodenext (not sure how you managed to get away with not doing this, as i even had to fix some upstream you definitely don't have)

e.g. there's also some type errors in rollup's plugins in nodenext which you can't have worked around as they're broken at the source. so i'd be curious how you did that

maybe you haven't had to tackle nodenext? and you're still using the older resolution strategy?

either way, i don't think you can just hope to rebase this branch onto yours. you first need to fully understand whats in your branch vs whats in my branch, so we know what exactly we're trying to take from each IMO.

43081j avatar Nov 22 '23 09:11 43081j

@koddsson where shall I post bugs found in the canary release?

bashmish avatar Nov 22 '23 17:11 bashmish

@koddsson where shall I post bugs found in the canary release?

I think just a issue would be great :)

koddsson avatar Nov 23 '23 08:11 koddsson

@koddsson where shall I post bugs found in the canary release?

I think just a issue would be great :)

What is your plan in releasing this? A separate issue I think is better for what is already released which is not the case here. Currently the storybook-builder is broken, I'd like to prevent a release of it with a broken version. IIRC you also wanted to bump a major version here, maybe even bump all 0.x.x to 1.x.x, I.m not sure it's a good idea for storybook-builder either.

bashmish avatar Nov 23 '23 09:11 bashmish