-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Build improvements #2076
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
Build improvements #2076
Conversation
`Built ${stats.hasWarnings() ? 'with warnings ' : ''}in ${took / | ||
1000}s.` | ||
) | ||
); | ||
console.log(); | ||
|
||
// console.log('File sizes after gzip:'); |
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.
We can start showing this again after we remove CopyWebpackPlugin
, I commented this because it was also showing all files copied by the plugin (+50,000 files), which made the build actually slow because of slow stdout.
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 think we can / should do something more involved as a separate step in the build process.
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.
Hmm, like a check? I'd agree with that, if you can include it in this PR that would be nice, if not we could uncomment this for now in the meantime.
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.
But how does that work, anyway, as in CI we don't have the previous build's assets?
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.
We do! We cache them between builds in CircleCI.
@@ -63,6 +50,12 @@ const ESLINT_PLUGIN_VUE_INDEX = `module.exports = { | |||
|
|||
const sepRe = `\\${path.sep}`; // path separator regex | |||
|
|||
const threadPoolConfig = { | |||
workers: 2, |
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.
Did this make it faster than cpu().length - 1
?
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 didn't see any obvious improvements, even when using thread-loader
vs. not using it, TBH. Can you maybe also test without it / with different numbers of workers and see what you get?
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.
Didn't see improvements either, let's keep it at 2.
f3681a2
to
39fcae1
Compare
package.json
Outdated
"**/react-codemirror2/**" | ||
"**/react-codemirror2/**", | ||
"app/webpack", | ||
"app/acorn-dynamic-import" |
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.
(Hacky?) Fix for webpack/webpack#8656 until acornjs/acorn#834 gets merged.
packages/app/scripts/start.js
Outdated
@@ -51,7 +54,7 @@ function formatMessage(message) { | |||
function clearConsole() { | |||
// This seems to work best on Windows and other systems. | |||
// The intention is to clear the output so you can focus on most recent build. | |||
process.stdout.write('\x1bc'); | |||
// process.stdout.write('\x1bc'); |
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.
@CompuIves Should we completely remove clearConsole()
? I kind of get its purpose, but on Windows all the previous output is completely lost, which is frustrating when you'd want to look at previous warnings / errors.
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.
Hmm, I see. I liked the clearConsole
but I think it makes sense to not do it in Windows. Maybe require('os').platform() === 'darwin'
check?
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.
Disabled by default on Windows and added CLEAR
environment variable to override default behavior.
Whooo, everything's back to green! 😎 |
// cm.toggleComment({ lineComment: '//' }); | ||
// }); | ||
// }, | ||
// 'Cmd-P': () => { |
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.
Would it be possible to enable this one? That will allow the file fuzzy search
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.
Re-added everything CodeMirror related back in.
- use CLEAR environment variable to override default behavior.
* [WIP] Build improvements. * Fix production build. * Extract static assets to config/build.js and use them in both dev and prod scripts. * Cleanup. * Update yarn.lock. * Update standalone-packages/vscode-textmate/package-lock.json. * Bump CircleCI cache keys version. * Revert "Update standalone-packages/vscode-textmate/package-lock.json." * Small improvements. * Update webpack to latest 4.35.0. * CodeMirror: remove auto-complete, so we're not bundling tern. * Remove unneeded gulp tasks. * Update webpack-dev-server to latest 3.7.2 and remove unused webpack-dev-middleware. * Show the updated module which triggers re-compiling. * Don't use thread-loader while linting. * Extract tern dep to codesandbox-deps and re-add CodeMirror bits. * Disable console clearing on Windows by default and - use CLEAR environment variable to override default behavior.
Description to come!copy-webpack-plugin
, replace it withcodesandbox-client/packages/app/scripts/start.js
Lines 192 to 196 in 0d5ef6b
happypack
, as it's in maintenance mode, and usethread-loader
insteadwebpack
to latest4.35.0
updatewebpack-dev-server
to3.2.1
(newer versions fail in a weird way, should maybe look more into that - reminder )start.js
andbuild.js
, show elapsed (re)build timespackage.json
scripts changesL.E.:
tern
, which hasacorn
as a dependency, which fails to build, see https://drone.ops.csb.dev/codesandbox/codesandbox-client/281we weren't loading the
codemirror-tern
chunk anyway, so I guess we were not using auto-complete? (although Ctrl-Space seems to work in the embeds?!)L.L.E.:
gulp
tasksL.L.L.E.:
webpack-dev-server
to latest3.7.2
; starting with3.3.0
(HMR plugin insertion for hot option and inline entry insertion in Server webpack/webpack-dev-server#1738) it auto-inserts the client into all entries based on theinline
option, which by default istrue
, that's why we needcodesandbox-client/packages/app/scripts/start.js
Line 269 in 51ee807
codesandbox-client/packages/app/config/webpack.dev.js
Lines 5 to 7 in bf0bd76
webpack-dev-middleware
Fixes #2069, #2070 .