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/#98 find loaders path separator agnostically #99

Conversation

fourpastmidnight
Copy link
Contributor

@fourpastmidnight fourpastmidnight commented Jun 2, 2022

  • 100% Test Coverage
  • Code is formatted with Prettier
  • No ESLint warnings
  • No security vulnerabilities in any NPM packages

Fixes #98.
Closes #84

Find Webpack CSS loaders using platform-independent path-separator logic.

fourpastmidnight and others added 2 commits July 7, 2023 21:01
I was using Ubuntu hosted in WSL with a project originally created on
Windows. I was actually running the project from Git for Windows (so, in
Windows). This resulted in some issues for craco-less.

Craco-less attempted to find loaders by first ascertaining the path
separator which should be used. So in Ubuntu in WSL, that would be `/`
since that's Linux. However, the project was installed and being run
from Windows, so the standard path separator is (usually) `\` (though,
PowerShell will work just fine with either `/` or `\`).

But, why should craco-less bother to match _exactly_ the path separator
being used to find a loader? It's unnecessary and brittle. Just so long
as the loader name appears between path separators should be good
enough. Heck, even `\<loader-name>/` shouldn't _really_ be a problem.
And even if it were, I'd be less worried about craco-less than something
else in my build toolchain failing as a result, at which time I would
then correct the issue. So again, matching against an exact path
separator is more complex, brittle, and unnecessary.

This commit replaces the searching logic with a function which uses amap
of loader name to regular expressions so that special-case path
separator logic is not required.
@kamronbatman kamronbatman force-pushed the fix/#98-find-loaders-path-separator-agnostically branch from 8376508 to 87f1e05 Compare July 8, 2023 04:02
@kamronbatman
Copy link
Collaborator

kamronbatman commented Jul 8, 2023

I rebased your changes into master. Looks like it needs test updates.
EDIT: Got that updated. If CI passes, we should be good to go.

@kamronbatman kamronbatman self-requested a review July 8, 2023 04:06
@kamronbatman kamronbatman merged commit 7c22523 into DocSpring:master Jul 8, 2023
20 checks passed
@fourpastmidnight
Copy link
Contributor Author

Awesome, thanks, I appreciate it!

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.

Find loaders path-separator agnostically
2 participants