Skip to content
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

WebPack builds for improved loading times #3889

Merged
merged 8 commits into from
Sep 7, 2019

Conversation

ianjfrosst
Copy link
Contributor

What this PR does / why we need it:
Add WebPack support via a gulp plugin

Which issue(s) this PR fixes
Partially fixes #3127

Special notes for your reviewer:
In my tests, loading times (Running Extensions -> Startup Activation) decrease from 2-4s to <1s.

The webpack-stream package is a little outdated, and raises some deprecation and vulnerability warnings. The best alternative would be to migrate fully from gulp to webpack, but that requires some discussion.

@TravisBuddy
Copy link

Travis tests have failed

Hey @ianjfrosst,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

View build log

if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; fi
diff --git a/webpack.config.js b/webpack.config.js
index f1250cd..4fe8e8a 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -14,24 +14,24 @@ const config = {
     path: path.resolve(__dirname, 'out'),
     filename: 'extension.js',
     libraryTarget: 'commonjs2',
-    devtoolModuleFilenameTemplate: '../[resource-path]'
+    devtoolModuleFilenameTemplate: '../[resource-path]',
   },
   devtool: 'source-map',
   externals: {
-    vscode: 'commonjs vscode' // the vscode-module is created on-the-fly and must be excluded. Add other modules that cannot be webpack'ed, 📖 -> https://webpack.js.org/configuration/externals/
+    vscode: 'commonjs vscode', // the vscode-module is created on-the-fly and must be excluded. Add other modules that cannot be webpack'ed, 📖 -> https://webpack.js.org/configuration/externals/
   },
   resolve: {
     // support reading TypeScript and JavaScript files, 📖 -> https://github.com/TypeStrong/ts-loader
-    extensions: ['.ts', '.js']
+    extensions: ['.ts', '.js'],
   },
   module: {
     rules: [
       {
         test: /\.ts$/,
         exclude: /node_modules/,
-        use: ['ts-loader']
-      }
-    ]
-  }
+        use: ['ts-loader'],
+      },
+    ],
+  },
 };
 module.exports = config;
Prettier Failed. Run [18:26:05] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[18:26:05] Starting 'forceprettier'...
[18:26:20] Finished 'forceprettier' after 14 s and commit changes to resolve.
TravisBuddy Request Identifier: 8bedb690-a4d2-11e9-bd96-c5bef65e495d

@TravisBuddy
Copy link

Travis tests have failed

Hey @ianjfrosst,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

View build log

if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; fi
The command "if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; fi" exited with 0.

$ npm run build

> vim@1.8.1 build /home/travis/build/VSCodeVim/Vim
> gulp build

[18:51:02] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[18:51:02] Starting 'build'...
[18:51:02] Starting 'prettier'...
[18:51:02] Finished 'prettier' after 19 ms
[18:51:02] Starting 'webpack'...
[18:51:02] Starting 'tslint'...
[18:51:39] Version: webpack 4.35.3
Built at: 2019-07-12 18:51:39
           Asset      Size  Chunks             Chunk Names
    extension.js   789 KiB       0  [emitted]  main
extension.js.map  3.13 MiB       0  [emitted]  main
Entrypoint main = extension.js extension.js.map

WARNING in ./node_modules/neovim/node_modules/colors/lib/colors.js 127:29-43
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/neovim/node_modules/colors/safe.js
 @ ./node_modules/neovim/node_modules/winston/lib/winston/config.js
 @ ./node_modules/neovim/node_modules/winston/lib/winston.js
 @ ./node_modules/neovim/lib/utils/logger.js
 @ ./node_modules/neovim/lib/host/NvimPlugin.js
 @ ./node_modules/neovim/lib/index.js
 @ ./src/neovim/neovim.ts
 @ ./src/state/vimState.ts
 @ ./src/mode/modeHandler.ts
 @ ./src/mode/modeHandlerMap.ts
 @ ./extension.ts

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/
[18:51:39] Finished 'webpack' after 37 s
[18:51:41] Finished 'tslint' after 39 s
[18:51:41] Starting 'commit-hash'...
[18:51:41] Finished 'commit-hash' after 43 ms
[18:51:41] Finished 'build' after 39 s
The command "npm run build" exited with 0.

$ npm test

> vim@1.8.1 test /home/travis/build/VSCodeVim/Vim
> node ./node_modules/vscode/bin/test

### VS Code Extension Test Run ###

Current working directory: /home/travis/build/VSCodeVim/Vim
Downloading VS Code 1.36.1 into .vscode-test/vscode-1.36.1.
Downloading VS Code from: https://update.code.visualstudio.com/1.36.1/linux-x64/stable
Downloaded VS Code 1.36.1
Running extension tests: /home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/code /home/travis/build/VSCodeVim/Vim/test --extensionDevelopmentPath=/home/travis/build/VSCodeVim/Vim --extensionTestsPath=/home/travis/build/VSCodeVim/Vim/test --locale=en
[main 2019-07-12T18:51:45.602Z] update#setState idle

bash: cannot set terminal process group (-1): Inappropriate ioctl for device

bash: no job control in this shell

nvm is not compatible with the "npm_config_prefix" environment variable: currently set to "/home/travis/.nvm/versions/node/v8.16.0"

Run `unset npm_config_prefix` to unset it.

[vscodevim.vim] Accessing a resource scoped configuration without providing a resource is not expected. To get the effective value for 'editor.wordSeparators', provide the URI of a resource or 'null' for any resource.

Error: Error: Cannot find module '/home/travis/build/VSCodeVim/Vim/test'
	at D._doHandleExtensionTests (/home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:751:543)
	at D._handleExtensionTests (/home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:750:852)
	at define._startExtensionHost._readyToStartExtensionHost.wait.then.then.then (/home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:752:138)
(node:6023) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

Tests exited with code: 1
npm ERR! Test failed.  See above for more details.
The command "npm test" exited with 1.

store build cache
changes detected (content changed, file is created, or file is deleted):\n/home/travis/.npm/_cacache/content-v2/sha1/0d/ee/3fed31fcd469618ce7342099fc1afa0bdb13
/home/travis/.npm/_cacache/content-v2/sha1/12/c2/5efe40a45e3c323eb8675a0a0ce57b22371f
/home/travis/.npm/_cacache/content-v2/sha1/19/58/70450f5a13192478df4bc3d23d2dea1907b6
/home/travis/.npm/_cacache/content-v2/sha1/1b/33/792e11e914a2fd6d6ed6447464444e5fa640
/home/travis/.npm/_cacache/content-v2/sha1/1b/b9/f314ef6b8baded13b549169b2a945eb68e4d
/home/travis/.npm/_cacache/content-v2/sha1/1f/16/e4aa22b04d1336b66188a66af3c600c3a66d
/home/travis/.npm/_cacache/content-v2/sha1/21/e0/abfaf6f2029cf2fafb133567a701d4135524
/home/travis/.npm/_cacache/content-v2/sha1/26/e6/1ed1422fb70dd42e6e36729ed51d855fe8d9
/home/travis/.npm/_cacache/content-v2/sha1/29/0c/bb232e306942d7d7ea9b83732ab7856f8285
/home/travis/.npm/_cacache/content-v2/sha1/38/38/e97cfc60521eb73c525a8e55bfdd9e2e28f1
/home/travis/.npm/_cacache/content-v2/sha1/3a/9a/20b8462523e447cfbc7e8bb80ed667bfc552
/home/travis/.npm/_cacache/content-v2/sha1/4d/aa/4d9db\n...
changes detected, packing new archive
uploading PR.3889/cache-linux-xenial-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--node-8.tgz
cache uploaded


Done. Your build exited with 1.
npm test
> vim@1.8.1 test /home/travis/build/VSCodeVim/Vim
> node ./node_modules/vscode/bin/test

### VS Code Extension Test Run ###

Current working directory: /home/travis/build/VSCodeVim/Vim
Downloading VS Code 1.36.1 into .vscode-test/vscode-1.36.1.
Downloading VS Code from: https://update.code.visualstudio.com/1.36.1/linux-x64/stable
Downloaded VS Code 1.36.1
Running extension tests: /home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/code /home/travis/build/VSCodeVim/Vim/test --extensionDevelopmentPath=/home/travis/build/VSCodeVim/Vim --extensionTestsPath=/home/travis/build/VSCodeVim/Vim/test --locale=en
[main 2019-07-12T18:51:45.602Z] update#setState idle

bash: cannot set terminal process group (-1): Inappropriate ioctl for device

bash: no job control in this shell

nvm is not compatible with the "npm_config_prefix" environment variable: currently set to "/home/travis/.nvm/versions/node/v8.16.0"

Run `unset npm_config_prefix` to unset it.

[vscodevim.vim] Accessing a resource scoped configuration without providing a resource is not expected. To get the effective value for 'editor.wordSeparators', provide the URI of a resource or 'null' for any resource.

Error: Error: Cannot find module '/home/travis/build/VSCodeVim/Vim/test'
	at D._doHandleExtensionTests (/home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:751:543)
	at D._handleExtensionTests (/home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:750:852)
	at define._startExtensionHost._readyToStartExtensionHost.wait.then.then.then (/home/travis/build/VSCodeVim/Vim/.vscode-test/vscode-1.36.1/VSCode-linux-x64/resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:752:138)
(node:6023) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

Tests exited with code: 1
npm ERR! Test failed.  See above for more details.
TravisBuddy Request Identifier: 1fdca020-a4d6-11e9-bd96-c5bef65e495d

@xconverge
Copy link
Member

@jpoon Any thoughts here? I am all for webpack and think it will be a great improvement to start up times, however I like our gulp infrastructure

@TravisBuddy
Copy link

Travis tests have failed

Hey @ianjfrosst,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

View build log

if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; fi
diff --git a/gulpfile.js b/gulpfile.js
index 368a860..47b5a06 100644
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -132,9 +132,7 @@ function updateVersion(done) {
 }
 
 function copyPackageJson() {
-  return gulp
-    .src('./package.json')
-    .pipe(gulp.dest('out'))
+  return gulp.src('./package.json').pipe(gulp.dest('out'));
 }
 
 gulp.task('tsc', function() {
@@ -247,7 +245,7 @@ gulp.task('run-test', function(done) {
 });
 
 gulp.task('build', gulp.series('prettier', gulp.parallel('webpack', 'tslint'), 'commit-hash'));
-gulp.task('test', gulp.series('tsc', copyPackageJson, 'run-test'))
+gulp.task('test', gulp.series('tsc', copyPackageJson, 'run-test'));
 gulp.task('changelog', gulp.series(validateArgs, createChangelog));
 gulp.task(
   'release',
Prettier Failed. Run [17:52:27] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[17:52:27] Starting 'forceprettier'...
[17:52:41] Finished 'forceprettier' after 14 s and commit changes to resolve.
TravisBuddy Request Identifier: 57fae930-a729-11e9-a3be-7d74802580dc

@jpoon
Copy link
Member

jpoon commented Jul 15, 2019

I haven't touched webpack in awhile but IIRC, they aren't exclusive. We can still continue using our gulp as a task runner for running tests, prettier, etc. and let webpack do it's thing with transpiling the TS and bundling? Or has the JS community deemed gulp un-cool like grunt and we should webpack everything?

@ianjfrosst
Copy link
Contributor Author

You can certainly run both. That's the way things are set up right now (on this branch). webpack is just another gulp task, like an alternative to tsc. The problem I'm having is that the webpacked output isn't suitable for running the tests. The recommended approach is to use tsc for the tests only, bu that causes a problem with the relative imports (src/ vs. out/src/).

@jpoon
Copy link
Member

jpoon commented Jul 16, 2019

The problem I'm having is that the webpacked output isn't suitable for running the tests.

Webpack newb here. How come?

The recommended approach is to use tsc for the tests only, bu that causes a problem with the relative imports (src/ vs. out/src/).

Yikes, doesn't that technically mean the transpiled JS code that we test against isn't the same as that we launch with?

@ianjfrosst
Copy link
Contributor Author

I'm also a webpack newb, so I might not be the best resource. The vscode developer docs have most of the info I'm going off of.

As I understand, webpack bases itself off the entrypoints of an application (in this case, extension.ts) and culls anything that isn't used from there. There are options for multiple entrypoints (with more confusing documentation), but even then, it may optimize them to into 2 separate files (in production mode). I don't believe there's a way to run the tests on the same transpiled JS code as the main output using webpack.

However, that is the way that all compiled projects run unit tests. It just depends on how much you trust the transpile+optimize+minify steps.

@ianjfrosst ianjfrosst changed the title [WIP] WebPack builds for improved loading times WebPack builds for improved loading times Jul 18, 2019
@ianjfrosst
Copy link
Contributor Author

@jpoon @xconverge what do you feel is left to do for this?

@xconverge
Copy link
Member

I feel really bad, basically I want this and support this, however we are a little light on time to maintain right now, and I am not up to speed with how webpack works really at all. I really apologize, we have been in a bit of a tough situation for a few months.

@J-Fields
Copy link
Member

@ianjfrosst Can you merge/rebase with latest master? This looks good to me.

@J-Fields
Copy link
Member

@ianjfrosst Could you update the branch? It'd be awesome to get this in the next release.

@J-Fields
Copy link
Member

J-Fields commented Sep 6, 2019

Can confirm that this does reduce measured startup time significantly (it does also feel a bit faster to start up). Also reduces .vsix size to ~33% of what it was. I'll do another sanity check later and then merge this in.

@J-Fields J-Fields self-assigned this Sep 6, 2019
@jpoon
Copy link
Member

jpoon commented Sep 6, 2019

Wow, that is amazing performance improvements! 🎉

@J-Fields J-Fields merged commit 1f80b2d into VSCodeVim:master Sep 7, 2019
@J-Fields
Copy link
Member

J-Fields commented Sep 7, 2019

Thanks @ianjfrosst!

rickpr pushed a commit to rickpr/Vim that referenced this pull request Oct 25, 2019
jpoon pushed a commit that referenced this pull request Oct 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Webpack Bundling
5 participants