jantimon / css-variable

Define CSSVariables in JS
https://css-variable.js.org
MIT License
89 stars 5 forks source link

Feature/handle windows paths #8

Closed Tyderion closed 2 years ago

Tyderion commented 2 years ago

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: https://github.com/rust-lang/rust/issues/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

martindisch commented 2 years ago

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.

jantimon commented 2 years ago

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

all steps run on windows and linux:
https://github.com/jantimon/css-variable/blob/6c00c48fec0a3e53cdb83c8d438be6556f16ebf3/.github/workflows/swc.yml#L13-L17

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:
    https://github.com/jantimon/css-variable/blob/6c00c48fec0a3e53cdb83c8d438be6556f16ebf3/test/swc/swc.test.js#L8-L10

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