-
Notifications
You must be signed in to change notification settings - Fork 1k
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
working branch for supporting windows contributors #6313
base: main
Are you sure you want to change the base?
Conversation
acf24cc
to
c1cdc69
Compare
add is-mingw use isMinGW to use proper slashes on windows git bash
c1cdc69
to
c92016b
Compare
@@ -140,6 +140,7 @@ | |||
"full-icu": "^1.3.0", | |||
"identity-obj-proxy": "^3.0.0", | |||
"ignore-styles": "^5.0.1", | |||
"is-mingw": "^2.0.0", |
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.
i don't need this.
i can do this instead:
const isMinGW = process.env.MSYSTEM === 'MINGW64' || process.env.MSYSTEM === 'MINGW32';
}); | ||
let szConfigImports = configImports.join('\n'); | ||
if (isMingw()) { | ||
storiesPath = storiesPath.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.
rather than doing .replace, we could do something more like...
> path.posix.join('a','b')
'a/b'
> path.win32.join('a','b')
'a\\b'
so
let curPath = path;
if (isMingw()) {
curPath = path.posix;
}
and then all the "join"/"relative" things would work
As a windows developer, I am unable to install RSP and contribute.
Steps to reproduce:
I have found a number of issues, mostly around the
build-icons
step.after much trial and error (mostly error), i have found that the culprit is that we are specifying
--extensions '.cjs,.js'
when we build each of the icon bundles. for some reason,babel-node
does not see this as a valid extension, and will actually simply ignore *.js anyway. Furthermore, this invalid extension value causes all other cli arguments to be ignored, and thuspresets @babel/env
is also ignored. this is where we get the cjs import transform from. Causing the above errors.after that i had an error starting story book, due to improper path separators. The launch failed in git bash. This is because in git bash we need forward slashes, not back slashes.
path
is not aware of this nuance and incorrectly gives us backslashes on every shell that detects aswin32
. I have used the packageis-mingw
to solve this in our preview.js.summarization of steps taken:
babel-node --extensions '.cjs,.js'
- babel automatically will use .cjs, and for some reason this was causing babel to ignore all other cmd line args, including--presets @babel/env
, which is where we get the cjs import transform on mac/linuxCloses (#6314)
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: