fix(cli): correct broken handling of Windows paths in inline command
#18488
Conversation
|
All contributors have signed the DCO. |
This comment was marked as resolved.
This comment was marked as resolved.
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| await replace({ | ||
| files: file.path, | ||
| from: REPLACE_REGEX, | ||
| glob: { windowsPathsNoEscape: true }, | ||
| to(_, match) { | ||
| return `@import '${ | ||
| isWin ? relativeImportPath.replace('\\', '/') : relativeImportPath |
There was a problem hiding this comment.
Option glob.windowsPathsNoEscape instructs replace-in-file to interpret Windows paths as-is and avoid escaping backslashes found in the value of the files argument, which on Windows leads to double escaping and yields an invalid glob pattern that will never match anything:
Example of a files value from the @carbon/grid build:
Successful @carbon/grid build after the fix:
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #18488 +/- ##
=======================================
Coverage 84.96% 84.96%
=======================================
Files 391 391
Lines 14490 14490
Branches 4741 4773 +32
=======================================
Hits 12312 12312
- Misses 2029 2030 +1
+ Partials 149 148 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@nchevsky thanks for putting this up! Does the full monorepo build work now or just the grid package? In either case what do you think about adding a new step to the ci workflow that runs on windows and ensures the build passes? It wouldn't have to be a required check, but would provide visibility into the status of windows support without having to manually test on a windows machine. |
@tay1orjones This PR only fixes That's a great idea about the build step! Unfortunately I'm spread very thin until April, but I'm happy to add that as we get closer to merging the last couple of path handling fixes. Does that sound reasonable? |
517562c
Fixes #16963 (closed with no resolution after the author disappeared).
Corrects broken handling of Windows paths in the
inlinecommand—used by the@carbon/gridbuild—by passing optionglob.windowsPathsNoEscapetoreplace-in-file.💡 This is the first in a series of patches to enable building all of Carbon on Windows natively, without WSL. 🚀
Testing / Reviewing
Build
@carbon/gridon a native Windows environment, or take my word for it: