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

test(esbuild/packgaejson): enable test_package_json_subpath_import_node_builtin_issue3485 #1186

Conversation

zhusjfaker
Copy link
Collaborator

@zhusjfaker zhusjfaker commented May 21, 2024

Description

https://github.com/evanw/esbuild/blob/main/internal/bundler_tests/bundler_packagejson_test.go#L2878

Please check the result of the #1185 issue before merging in this MR.

#1185

#1185

Confirm that the pack is what the rolldown user wants.

import { default as fs } from "#fs";
import { default as http } from "#http";

// entry.js
fs.readFileSync();
http.createServer();

even "#fs" will cause the js to crash

Copy link

netlify bot commented May 21, 2024

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 3774ba1
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/664d686a94f94c0008216b8a

@hyf0
Copy link
Member

hyf0 commented May 21, 2024

Please rename the title to test(esbuild/packgaejson): enable test_package_json_subpath_import_node_builtin_issue3485

@hyf0
Copy link
Member

hyf0 commented May 21, 2024

The result is not right. However, it's ok we merge them first and solve #1185 later.

@zhusjfaker zhusjfaker force-pushed the fix/test_package_json_subpath_import_node_builtin_issue3485 branch from 6f98f70 to 684487f Compare May 21, 2024 09:56
@zhusjfaker
Copy link
Collaborator Author

The result is not right. However, it's ok we merge them first and solve #1185 later.

I'll make an issue of the progress of all esbuild packagejson cases, so that we can track the progress of all esbuild packagejson cases by table, and the reasons for the obstacles and related merge-ins in the MR and other related issue discussions.

@zhusjfaker
Copy link
Collaborator Author

As of this MR, I've finished migrating all the cases that were successfully packaged by esbuild, but there are still a few cases that throw exceptions, so I'll reflect the progress of the migration in the overall issue.

@zhusjfaker zhusjfaker force-pushed the fix/test_package_json_subpath_import_node_builtin_issue3485 branch from 684487f to b483635 Compare May 21, 2024 10:01
hyf0
hyf0 previously approved these changes May 21, 2024
@hyf0 hyf0 added this pull request to the merge queue May 21, 2024
@hyf0 hyf0 removed this pull request from the merge queue due to a manual request May 21, 2024
@hyf0 hyf0 dismissed their stale review May 21, 2024 10:12

Please rename the title to test(esbuild/packgaejson): enable test_package_json_subpath_import_node_builtin_issue3485

@zhusjfaker zhusjfaker changed the title fix: test_package_json_subpath_import_node_builtin_issue3485 test(esbuild/packgaejson): enable test_package_json_subpath_import_node_builtin_issue3485 May 22, 2024
@zhusjfaker
Copy link
Collaborator Author

The result is not right. However, it's ok we merge them first and solve #1185 later.

done

@zhusjfaker zhusjfaker force-pushed the fix/test_package_json_subpath_import_node_builtin_issue3485 branch from b483635 to 1a99851 Compare May 22, 2024 03:00
@zhusjfaker zhusjfaker force-pushed the fix/test_package_json_subpath_import_node_builtin_issue3485 branch from 1a99851 to 3774ba1 Compare May 22, 2024 03:37
Copy link
Member

@hyf0 hyf0 left a comment

Choose a reason for hiding this comment

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

Thanks

@hyf0 hyf0 added this pull request to the merge queue May 22, 2024
Merged via the queue into rolldown:main with commit 5b9c057 May 22, 2024
18 checks passed
@zhusjfaker zhusjfaker deleted the fix/test_package_json_subpath_import_node_builtin_issue3485 branch May 24, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants