这是indexloc提供的服务，不要输入任何密码
Skip to content

feat!: add ResolveError::Builtin::is_runtime_module #272

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 22, 2024
Merged

feat!: add ResolveError::Builtin::is_runtime_module #272

merged 3 commits into from
Oct 22, 2024

Conversation

@IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Oct 21, 2024 •
edited by Boshen
Loading

ResolveError::Builtin::is_runtime_module can be used to determine whether the original specifier was prefixed with node: or not.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 21, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #272 will not alter performance

Comparing IWANABETHATGUY:feat/get-builtin-module-original-specifier (8c3c8e5) with main (efe9f4e)

Summary

✅ 3 untouched benchmarks

Boshen
Boshen previously requested changes Oct 21, 2024
View reviewed changes
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

We need tests in src/tests/builtins.rs

@ematipico
Copy link
Contributor

ematipico commented Oct 21, 2024

This is a breaking change, it changes the shape of a public error.

@IWANABETHATGUY
Copy link
Contributor Author

IWANABETHATGUY commented Oct 21, 2024

This is a breaking change, it changes the shape of a public error.

Yeah, the breaking change may not be avoided. it is used for scenarios like bundler or others want to get the original builtin module specifier because always returns
prefix node: is not going to work when the node is under 14

Minimum Version Required: The node: prefix is supported starting from Node.js 12.20.0 and 14.13.1.

@IWANABETHATGUY
Copy link
Contributor Author

IWANABETHATGUY commented Oct 21, 2024

We need tests in src/tests/builtins.rs

I think there are plenty of tests that cover this, https://github.com/oxc-project/oxc-resolver/pull/272/files#diff-945bb3291582dde54a233e5b184b7917ced686114d546a5c47b8ba2eb96baf36R87-R98

@IWANABETHATGUY IWANABETHATGUY force-pushed the feat/get-builtin-module-original-specifier branch from 201f70c to 249bf10 Compare October 21, 2024 10:20
@Boshen Boshen changed the title feat: get builtin module original specifier feat!: add ResolveError::Builtin::prefixed_with_node_colon Oct 21, 2024
@Boshen Boshen enabled auto-merge (squash) October 21, 2024 10:42
Boshen
Boshen reviewed Oct 21, 2024
View reviewed changes
@Boshen Boshen changed the title feat!: add ResolveError::Builtin::prefixed_with_node_colon feat!: add ResolveError::Builtin::is_runtime_module Oct 22, 2024
@Boshen Boshen merged commit 3a71a3e into oxc-project:main Oct 22, 2024
18 checks passed
@Boshen Boshen mentioned this pull request Oct 22, 2024
Closed
@IWANABETHATGUY IWANABETHATGUY deleted the feat/get-builtin-module-original-specifier branch October 22, 2024 04:32
This was referenced Oct 22, 2024
Closed
Closed
Closed
Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Boshen Boshen Boshen left review comments

+1 more reviewer

@ematipico ematipico ematipico left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@IWANABETHATGUY @ematipico @Boshen