Skip to content

Feature/handle windows paths #8

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

Merged
merged 5 commits into from
Jun 6, 2022

Conversation

Tyderion
Copy link
Collaborator

@Tyderion Tyderion commented Jun 5, 2022

Due to the rust Path module (and all other similar crates I've found) not being able to handle windows paths in the webassembly version (seems to work only on windows, see: rust-lang/rust#66621) I've implemented a simple naive conversion of filepaths to be posix-like even if the input is a windows path

might fix #6

@jantimon I was not able to test this in our next-js project, as npm via file:// did not actually work for imports, but the plugin did run

@Tyderion Tyderion linked an issue Jun 5, 2022 that may be closed by this pull request
@martindisch
Copy link
Collaborator

Oh, that's too bad. I haven't tested any of this, but if the compiler thinks the target is a POSIX environment when compiling for Wasm (which would make sense), then all the crates using conditional compilation like path-slash fall apart for our use case.

I just did some minor cleanup that I didn't get the chance to do before the previous PR got completed, but other than that have nothing to add. I'm almost certain there's some cases our implementation here doesn't handle, but it's just as likely that we won't hit them. I'd say go for it and try it out.

@martindisch martindisch force-pushed the feature/handle-windows-paths branch from 188dbf4 to f621dc4 Compare June 5, 2022 20:22
@jantimon jantimon merged commit 1f2fec0 into jantimon:main Jun 6, 2022
@jantimon
Copy link
Owner

jantimon commented Jun 6, 2022

thanks @Tyderion @martindisch merged and published - I also added a new github action test workflow.

all steps run on windows and linux:

cross-os:
strategy:
matrix:
node-version: [13.x]
os: [ubuntu-latest, windows-latest]

the workflow runs the following steps:

  1. Compile the SWC WASM Plugin
  2. Run all rust unit tests
  3. Run @swc/jest with the css-variable plugin
  4. Verify that the hash matches:
    expect(foo.name).toMatchInlineSnapshot(`"--foo--_llxb0"`);
    expect(bar.name).toMatchInlineSnapshot( `"--bar--_llxb1"`);
    expect(baz.name).toMatchInlineSnapshot( `"--baz--_llxb2"`);

here are the results: https://github.com/jantimon/css-variable/runs/6758811224?check_suite_focus=true

@Tyderion Tyderion deleted the feature/handle-windows-paths branch June 6, 2022 18:54
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.

swc plugin generates inconsistent hashes
3 participants