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

fix(es/module): Fix jsc.paths on Windows #6930

Merged
merged 9 commits into from Feb 11, 2023
Merged

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Feb 10, 2023

@kdy1 kdy1 added this to the Planned milestone Feb 10, 2023
@kdy1 kdy1 self-assigned this Feb 10, 2023
@kdy1
Copy link
Member Author

kdy1 commented Feb 10, 2023

Investigation

From test suite

   INFO  jsc.paths, base_url: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input
    at crates\swc_ecma_loader\src\resolvers\tsc.rs:54
    in Compiler.parse
    in parse_js_as_input
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\config\index.ts)

   INFO  Done in 2.7276ms, kind: "perf"
    at crates\swc_timer\src\lib.rs:32
    in Compiler.parse
    in parse_js_as_input
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\config\index.ts)

  DEBUG  Renaming `config#1` to `config`
    at crates\swc_ecma_transforms_base\src\rename\analyzer\scope.rs:183
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\config\index.ts)

   INFO  Done in 652µs, kind: "perf"
    at crates\swc_timer\src\lib.rs:32
    in Compiler.print
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\config\index.ts)

  DEBUG  compare_to_file: failed to canonicalize outfile path `\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\output\index.map`: Os { code: 2, kind: NotFound, message: "지정된 파일을 찾을 수 없습니다." }
    at crates\testing\src\output.rs:115

File: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts
   INFO  Done in 1.0332ms, kind: "perf"
    at crates\swc_timer\src\lib.rs:32
    in Compiler.parse
    in parse_js_as_input
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

  DEBUG  invoking resolver
    at crates\swc_ecma_transforms_module\src\path.rs:158
    in resolve_import with base: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts, module_specifier: @/config
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

  DEBUG  non-relative import
    at crates\swc_ecma_loader\src\resolvers\tsc.rs:131
    in tsc.resolve with base_url: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input, base: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts, src: @/config
    in resolve_import with base: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts, module_specifier: @/config
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

  TRACE  to_specifier: orig_ext=None
    at crates\swc_ecma_transforms_module\src\path.rs:119
    in resolve_import with base: \\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts, module_specifier: @/config
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

  DEBUG  Renaming `config#3` to `config`
    at crates\swc_ecma_transforms_base\src\rename\analyzer\scope.rs:183
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

  DEBUG  Renaming `main#3` to `main`
    at crates\swc_ecma_transforms_base\src\rename\analyzer\scope.rs:183
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

   INFO  Done in 600.6µs, kind: "perf"
    at crates\swc_timer\src\lib.rs:32
    in Compiler.print
    in process_js_inner
    in process_js_with_custom_pass
    in process_js_file with fm: SourceFile(\\?\C:\Users\MSI\projects\swc-win\crates\swc\tests\fixture\issues-6xxx\6782\1\input\src\index.ts)

base is UNC

Real repo

path is src/index.ts

@kdy1 kdy1 marked this pull request as ready for review February 10, 2023 06:12
kodiakhq[bot]
kodiakhq bot previously approved these changes Feb 10, 2023
Copy link
Member Author

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

swc-bump:

  • swc

@kdy1 kdy1 changed the title fix(es/module): Fix interop of yarn and jsc.paths on Windows fix(es/module): Fix jsc.paths on Windows Feb 10, 2023
@kdy1
Copy link
Member Author

kdy1 commented Feb 10, 2023

@realtimetodie Can you send PR for adding tests for #6716? So I can ensure those are working while fixing #6930

@realtimetodie
Copy link
Contributor

I can confirm that between b451fa9 and e332dff (now) the module resolution has changed and the previous behavior can no longer be reproduced.

I also confirmed that the Bazel test fails prior to b451fa9 due to path canonicalization

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //examples/paths:test_test
-----------------------------------------------------------------------------
5c5
< const _moduleA = require("../../../../../../../../.cache/bazel/_bazel_kyoko/d981ac4e564436252b4ad023c1ebd769/sandbox/linux-sandbox/35/execroot/aspect_rules_swc/examples/paths/src/modules/moduleA");
---
> const _moduleA = require("./modules/moduleA");
FAIL: files "examples/paths/src/index.js" and "examples/paths/expected.js" differ. 

@//examples/paths:expected.js is out of date. To update this file, run:

    bazel run //examples/paths:test

To confirm this behavior, I created an integration test for Bazel using symbolic links in. The tree is checked out at that fails due to path canonicalization

@realtimetodie
Copy link
Contributor

I tested the binary manually with Bazel and the tests pass.

@realtimetodie
Copy link
Contributor

@kdy1 I added a Bazel test in realtimetodie@1dc54c8 that verifies symbolic link support.

@kdy1
Copy link
Member Author

kdy1 commented Feb 11, 2023

@realtimetodie Can you create a PR towards swc main?

@realtimetodie
Copy link
Contributor

@kdy1 #6933

@kdy1
Copy link
Member Author

kdy1 commented Feb 11, 2023

Thank you!

@kdy1 kdy1 enabled auto-merge (squash) February 11, 2023 15:19
Copy link
Collaborator

@swc-bot swc-bot left a comment

Choose a reason for hiding this comment

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

Automated review comment generated by auto-rebase script

Copy link
Member Author

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

swc-bump:

  • swc

@kdy1 kdy1 merged commit 1ec161a into swc-project:main Feb 11, 2023
@kdy1 kdy1 deleted the issue-6782 branch February 11, 2023 16:27
@kdy1 kdy1 modified the milestones: Planned, v1.3.36 Feb 21, 2023
@swc-project swc-project locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Issue with yarn and path building
3 participants