-
Notifications
You must be signed in to change notification settings - Fork 362
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
Feat/create react app v4 #229
Conversation
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.
Sorry haven't seen the comment yet
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.
Apologies I merged to quickly. Let's discuss your question over a subsequent PR
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.
See response
|
||
fs.mkdirpSync(assetsdir); | ||
|
||
const indexHtml = [ |
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.
Might make sense to store this in a separate file and fs.readFile
'export default reportWebVitals;', | ||
]; | ||
|
||
fs.mkdirpSync(srcdir); |
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.
See the PR @mbonig is working on. I think we'll have something like SampleDir
which will accept multiple files.
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 can go back and clean this up once that PR is in!
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.
Thanks
*Issue #, if available:* N/A *Description of changes:* Fix currently failing build caused by merging of #228 and #229 to master Also fixed categorization of some commands By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Addresses some comments from #229 I ran `diff` against the two folders to check the same files are generated.
Issue #, if available: 158
Description of changes:
react
react-ts
allowSyntheticDefaultImports
Something to discuss - should the output files use:
fs.writeFileSync(path.join(srcdir, 'logo.svg'), logoSvg.join('\n'));
- just joins the string arraynew ReactAppTest(this.project, path.join(srcdir, 'App.test.' + this.fileExt));
- extendsFileBase
I left both options as examples in the code. Either to swap either way.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.