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

properly output esm #1331

Merged
merged 3 commits into from
Jun 19, 2024
Merged

properly output esm #1331

merged 3 commits into from
Jun 19, 2024

Conversation

turbocrime
Copy link
Contributor

@turbocrime turbocrime commented Jun 18, 2024

uses tsc-alias to generate correct import paths for esm support

this may have been related to issues @VanishMax was experiencing

this should permit esm resolution to work properly for the packages' internal modules

edit: okay, tsc-alias doesn't completely solve the issue -

bundlers are fine now, but anything that needs to use node module resolution (vitest, many consumers) will encounter problems because our packages currently use nonstandard module resolution to identify imports from external dependencies.

the tsc-alias tool is only indended for internal paths.

probably we should just comply with esm path spec internally, but that would require changing nearly every import in the codebase, so this PR now combines tsc-alias and changes imports to name external 'js' files that are not resolved via package.json exports.

this mainly just means pb types imports now require a 'js' extension, because those generated packages don't actually define any exports or main configuration.

there's hundreds of pb type imports, so it's a large diff. but this means we don't have to configure a bundler for every package, and the packages should have much better support for many consumers.

edit again:

instead of literally modifying imports, now mutating in a more custom way

this solution is bad. eventually a larger linter/bundler reconfig should happen

Copy link

changeset-bot bot commented Jun 18, 2024

🦋 Changeset detected

Latest commit: dfce529

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
@penumbra-zone/transport-chrome Major
@penumbra-zone/transport-dom Minor
@penumbra-zone/perspective Major
@penumbra-zone/protobuf Minor
@penumbra-zone/services Major
@repo/tsconfig Minor
@penumbra-zone/bech32m Minor
@penumbra-zone/getters Major
@penumbra-zone/storage Major
@penumbra-zone/client Major
@penumbra-zone/crypto-web Major
@penumbra-zone/query Major
@penumbra-zone/types Major
@penumbra-zone/wasm Major
minifront Minor
@repo/ui Major
node-status Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Contributor

@TalDerei TalDerei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so tsc-alias ensures that module paths are correctly resolved to relative paths? assuming this is related to the export name / path mangling that was happening?

@turbocrime turbocrime requested review from TalDerei and a team June 18, 2024 23:48
@turbocrime turbocrime merged commit 9b3f561 into main Jun 19, 2024
6 checks passed
@turbocrime turbocrime deleted the turbocrime/esm-extensions branch June 19, 2024 05:27
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.

2 participants