-
Notifications
You must be signed in to change notification settings - Fork 100
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
chore: fix build on Windows #1606
Conversation
@@ -11,7 +11,7 @@ | |||
"homepage": "https://instructure.design", | |||
"bugs": "https://github.com/instructure/instructure-ui/issues", | |||
"scripts": { | |||
"prestart": "npm run build:scripts:ts && node -e 'import(\"./lib/build-docs.mjs\").then(file => file.buildDocs());'\n", |
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.
Windows does not like such a fancy command at all :/ had to move it to a file
@@ -42,9 +43,9 @@ export default { | |||
}, | |||
handler: async (argv) => { | |||
const configFile = path.join(process.cwd(), argv.config) | |||
const config = await import(configFile) | |||
const configFileURL = pathToFileURL(configFile) | |||
const config = await import(configFileURL) |
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.
import()
for files must begin with file://
, this is added by this built in helper
|
packages/__docs__/build-docs.mjs
Outdated
// scripts must be built from .ts files before this is run | ||
import {buildDocs} from './lib/build-docs.mjs' | ||
|
||
buildDocs() |
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.
was this file created in windows? I think this file ending is not compatible with unix EOF. I think editing this file in macos should solve this
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.
also the same file name with lib/build-docs.mjs
is a bit confusion. I'm wondering if there's that file already, can't that one be run instead?
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.
There is a way not to create a new file and not use fancy syntax in package.json
but its also ugly: https://stackoverflow.com/a/75760840 What do you think?
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.
fixed it, now it doesn't need an extra file
01d4391
to
eddc67e
Compare
eddc67e
to
b468d3a
Compare
not fully fixed yet, cant commit due to conventional-changelog/commitlint#4106
what works:
bootstrap
dev
test:vitest
almost (1 test fails for some reason)