-
-
Notifications
You must be signed in to change notification settings - Fork 382
fix(output-file-system): change from backslash to forward slash in join #804
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #804 +/- ##
=======================================
Coverage 99.22% 99.23%
=======================================
Files 10 10
Lines 259 261 +2
Branches 83 83
=======================================
+ Hits 257 259 +2
Misses 2 2
Continue to review full report at Codecov.
|
Please provide a problem |
Also this code for webpack@4 |
@@ -27,7 +27,9 @@ export default function setupOutputFileSystem(context) { | |||
} else { | |||
outputFileSystem = createFsFromVolume(new Volume()); | |||
// TODO: remove when we drop webpack@4 support | |||
outputFileSystem.join = path.join.bind(path); | |||
const newPath = Object.assign({}, path); | |||
newPath.join = (...files) => path.join(...files).replace(/[\\/]+/g, '/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using regexp to change \
on /
is bad idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newPath.join = (...files) => path.join(...files).replace(/[\\/]+/g, '/'); | |
newPath.join = (...files) => path.join(...files).replaceAll('\\', '/'); |
Would this be better?
@alexander-akait Can you please let me know what more explanations are needed? |
Please show me a problem why you need to change it |
Simple reproducible test repo |
@alexander-akait Sorry i think i was mistaken |
This PR contains a:
Motivation / Use-Case
In
webpack
, if there is ajoin
method ofoutputFileSystem
, use thejoin
method. However, if you use thejoin
method inwindows os
, a problem occurs when the path is relative.Ex)
before
join
:/app/js
after
join
:\app\join
If it changes as above, the following error occurs.
This PR solves that problem.
Breaking Changes
The
join
method ofoutputFileSystem
returns all backslashes replaced with slashes.Additional Info
https://github.com/webpack/webpack/blob/master/lib/util/fs.js#L133-L145