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

Remove unnecessary root re-exports #19742

Closed
wants to merge 8 commits into from
Closed

Remove unnecessary root re-exports #19742

wants to merge 8 commits into from

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Nov 3, 2022

Issue:

These files were a bit of a crutch in the days before package.json exports maps.

What I did

I'm opening this PR as a bit of an experiment to see if CI will pass.

We are currently using moduleResolution: node in our typescript project, which I recently learned means "node11" (the TS team is planning to rename it). Node 11 does not support package.json exports. In order to support it, we could change our moduleResolution to "Node16" or "nodenext", but that also implies the use of ESM, and we're not writing code that's used directly in node, but rather is being bundled using esbuild (via tsup). There is not currently a module resolution in typescript for bundlers (see microsoft/TypeScript#50152 for a great description of the current problems).

This also adds a typesVersions field to a package.json in order to get tsup to build correctly. This is one of the options for compatibility laid out in https://github.com/andrewbranch/example-subpath-exports-ts-compat. It does not support bundling with parcel or browserify, but I think we don't need to worry about that, since we support webpack and vite, and use esbuild internally.

How to test

  • CI

@IanVS IanVS added the maintenance User-facing maintenance tasks label Nov 3, 2022
# Conflicts:
#	code/addons/highlight/preview.js
#	code/addons/interactions/preview.js
#	code/addons/outline/preview.js
@IanVS
Copy link
Member Author

IanVS commented Nov 10, 2022

This is blocked until we can upgrade to jest 28+. Done!

@ndelangen
Copy link
Member

Good try, unless you're interested in making this truly happen, I suggest we close this for now, and maybe I'll revisit this after 7.0.

@ndelangen ndelangen closed this Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants