Skip to content

Use path.posix.join for relevant paths - #878

Merged
fredrikekelund merged 1 commit into
trunkfrom
f26d/fix-playground-internal-paths-on-windows
Feb 3, 2025
Merged

Use path.posix.join for relevant paths#878
fredrikekelund merged 1 commit into
trunkfrom
f26d/fix-playground-internal-paths-on-windows

Conversation

@fredrikekelund

@fredrikekelund fredrikekelund commented Feb 3, 2025

Copy link
Copy Markdown
Contributor

Related issues

Proposed Changes

This PR fixes a regression from #845. In that PR, we moved all mu-plugins but the SQLite one to be mounted in Playground's internal mu-plugins directory to give back more control to users over the regular mu-plugins directory.

The problem is that we use path.join for the paths passed to php.writeFile. The php.writeFile seems to expect Unix-style paths, but path.join is platform-aware, meaning we get Windows-style paths on Windows.

This PR fixes that problem by using path.posix.join instead.

Testing Instructions

Ensure that starting a server works as expected on Windows

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@fredrikekelund
fredrikekelund requested review from a team and bgrgicak February 3, 2025 14:11
@fredrikekelund fredrikekelund self-assigned this Feb 3, 2025

@nightnei nightnei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works well with MacOS and Windows 👍 Thanks

@fredrikekelund
fredrikekelund merged commit ad2e2c5 into trunk Feb 3, 2025
@fredrikekelund
fredrikekelund deleted the f26d/fix-playground-internal-paths-on-windows branch February 3, 2025 14:43

@sejas sejas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the quick fix! ❤️

@bgrgicak

bgrgicak commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

Thanks for fixing this @fredrikekelund TIL!

Should we add a linter rule to discourage the use of path.join in the future?

@fredrikekelund

Copy link
Copy Markdown
Contributor Author

Should we add a linter rule to discourage the use of path.join in the future?

In other places (anything related to the computer's regular file system), the platform-awareness of path.join is exactly what we want. This case — where we want Unix-style paths on all platforms — is the more uncommon one.

It's worth noting that since #839, we're running a CI job with unit tests on Windows again. Hopefully, that should help catch similar issues in the future 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants