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

Migrate to Node16 resolution #186

Merged
merged 21 commits into from
Mar 25, 2024
Merged

Migrate to Node16 resolution #186

merged 21 commits into from
Mar 25, 2024

Conversation

paul-sachs
Copy link
Collaborator

@paul-sachs paul-sachs commented Sep 29, 2023

Updating everything to use node16 resolutions. This guarantees (via the tsc compiler) that the files we output match the expected import format for downstream tools. This also brings us into compliance with module resolution for node 16+, which is the minimum version of node we support and is the recommended value for libraries.

See detailed explanation here

@paul-sachs paul-sachs marked this pull request as draft October 5, 2023 13:13
@paul-sachs paul-sachs marked this pull request as ready for review October 10, 2023 19:03
@nicksnyder
Copy link
Member

nicksnyder commented Oct 11, 2023

Can you update PR description to explain why we are moving to node16 resolution?

packages/connect-query/tsconfig.json Outdated Show resolved Hide resolved
packages/protoc-gen-connect-query/tsconfig.build.json Outdated Show resolved Hide resolved
packages/connect-query/src/stable-hash.js Outdated Show resolved Hide resolved
packages/connect-query/package.json Outdated Show resolved Hide resolved
@paul-sachs
Copy link
Collaborator Author

@timostamm @smaye81 resurrecting this PR to migrate to node16.

paul-sachs added a commit that referenced this pull request Dec 14, 2023
Fixes #298 (again)

Missed another file. All these will automatically be caught once #186 is
merged.
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

@paul-sachs, we just switched protobuf-es to build with Node16 module resolution, see bufbuild/protobuf-es#754. It's not released yet, and connect-es has not been updated yet (see connectrpc/connect-es#887).

The changes here look good to me. Just left a couple of comments.

packages/connect-query/tsconfig.build.json Outdated Show resolved Hide resolved
tsconfig.base.json Show resolved Hide resolved
packages/connect-query/package.json Outdated Show resolved Hide resolved
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

LGTM!

@paul-sachs paul-sachs merged commit 865cd1c into main Mar 25, 2024
7 checks passed
@paul-sachs paul-sachs deleted the psachs/node16-resolution branch March 25, 2024 17:16
@paul-sachs paul-sachs mentioned this pull request May 10, 2024
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.

3 participants