Skip to content

fix: support win32 paths when transforming compiled code#6

Merged
christian-bromann merged 1 commit intostencil-community:mainfrom
acerspyro:bugfix/win32-support
Jun 11, 2025
Merged

fix: support win32 paths when transforming compiled code#6
christian-bromann merged 1 commit intostencil-community:mainfrom
acerspyro:bugfix/win32-support

Conversation

@acerspyro
Copy link
Copy Markdown
Contributor

@acerspyro acerspyro commented Jun 11, 2025

I encountered a critical bug on Windows where import paths are parsed very wrong when being processed.

This means anyone using stencil + storybook on Windows will get an import error when loading the component in Storybook

Essentially, transformCompiledCode() would replace "./index.js" in, for example, dist/components/my-component.js with:

/Users/veryloud/Projects/my-project/C:UsersveryloudProjectsmy-projectdistcomponents/index.js

This fix makes sure the path method that is correct for the user's system is used.

@christian-bromann
Copy link
Copy Markdown
Member

Thanks for the contribution! Anything missing for this to be ready for review?

@acerspyro
Copy link
Copy Markdown
Contributor Author

Just missing working unit tests, I couldn't mock the platform for path as node sets it on startup, I'm tempted to send it for review sans tests and open another PR to figure that out.

@acerspyro acerspyro force-pushed the bugfix/win32-support branch from 7729e31 to a64a94e Compare June 11, 2025 04:22
@acerspyro acerspyro marked this pull request as ready for review June 11, 2025 04:22
@valadas
Copy link
Copy Markdown

valadas commented Jun 11, 2025

I can confirm this fixes the issue on windows, thanks a lot @acerspyro

@acerspyro acerspyro force-pushed the bugfix/win32-support branch from a64a94e to ec95805 Compare June 11, 2025 06:01
@acerspyro
Copy link
Copy Markdown
Contributor Author

Nice, looks like the pipeline is passing!

I'm testing a local build with a colleague, it should be good to merge after if you're satisfied with it :)

Copy link
Copy Markdown
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Great contribution 👏 thanks a lot!

@christian-bromann christian-bromann merged commit a7758ce into stencil-community:main Jun 11, 2025
10 checks passed
@acerspyro
Copy link
Copy Markdown
Contributor Author

acerspyro commented Jun 11, 2025

I can confirm that it works on our end!

@christian-bromann thank you!

@christian-bromann
Copy link
Copy Markdown
Member

Thank YOU!

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.

3 participants