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.

Sign up for GitHub

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

Jump to bottom

Webpack3 (no es6 changes) #4800

Closed
wants to merge 12 commits into from
Closed

Webpack3 (no es6 changes) #4800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3e7561f
Update to webpack3
MTRNord Aug 15, 2017
7ae2548
Force olm to be loaded before all other .js files.
MTRNord Aug 15, 2017
809cb33
Change install order in tests to prevent NPM fail
MTRNord Aug 15, 2017
46b784b
Fix Modal lacking needed functions
MTRNord Aug 16, 2017
6a2960f
Fix typo on import
MTRNord Aug 16, 2017
aca13d4
Merge branch 'develop' into webpack3_noES6
MTRNord Apr 9, 2018
ddee08b
Update babel-loader version
MTRNord Apr 9, 2018
33b9d5e
Merge branch 'develop' into webpack3_noES6
MTRNord Apr 20, 2018
68c2c5c
Fix wrongly resolved merge conflict
MTRNord Apr 20, 2018
1670703
Update postcss and its dependencies for webpack3
MTRNord Apr 20, 2018
07a8505
Fix webpack to fail with @imports
MTRNord Apr 26, 2018
f8e4263
Use include instead including all scss of node_modules
MTRNord Apr 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ addons:
install:
# clone the deps with depth 1: we know we will only ever need that one
# commit.
- scripts/fetch-develop.deps.sh --depth 1 && npm install
- npm install && scripts/fetch-develop.deps.sh --depth 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vaguely remember asking you about this back when you submitted the PR... if I did, I can't remember the conclusion though. Why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was the CI being stupid. It works this way not the other way. But I have to admit I do not know if this still is required

Copy link
Contributor Author

@MTRNord MTRNord Apr 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was propably the symlink issue that npm has which later got finished

7 changes: 4 additions & 3 deletions karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ const olm_entry = webpack_config.entry['olm'];
delete webpack_config['entry'];

// add ./test as a search path for js
webpack_config.module.loaders.unshift({
test: /\.js$/, loader: "babel",
webpack_config.module.rules.unshift({
test: /\.js$/, use: "babel-loader",
include: [path.resolve('./src'), path.resolve('./test')],
});

Expand All @@ -46,8 +46,9 @@ webpack_config.module.noParse.push(/sinon\/pkg\/sinon\.js$/);
// ?
webpack_config.resolve.alias['sinon'] = 'sinon/pkg/sinon.js';

webpack_config.resolve.root = [
webpack_config.resolve.modules = [
path.resolve('./test'),
"node_modules"
];

webpack_config.devtool = 'inline-source-map';
Expand Down
29 changes: 14 additions & 15 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@
"build:modernizr": "modernizr -c .modernizr.json -d src/vector/modernizr.js",
"build:compile": "npm run reskindex && babel --source-maps -d lib src",
"build:bundle": "cross-env NODE_ENV=production webpack -p --progress --bail",
"build:bundle:dev": "webpack --optimize-occurence-order --progress --bail",
"build:bundle:dev": "webpack --progress --bail",
"build:electron": "npm run clean && npm run build && npm run install:electron && build -wml --ia32 --x64",
"build": "npm run reskindex && npm run build:res && npm run build:bundle",
"build:dev": "npm run reskindex && npm run build:res && npm run build:bundle:dev",
"dist": "scripts/package.sh",
"install:electron": "install-app-deps",
"electron": "npm run install:electron && electron .",
"start:res": "node scripts/copy-res.js -w",
"start:js": "webpack-dev-server --output-filename=bundles/_dev_/[name].js --output-chunk-file=bundles/_dev_/[name].js -w --progress",
"start:js": "webpack-dev-server --output-filename=bundles/_dev_/[name].js --output-chunk-filename=bundles/_dev_/[name].js -w --progress",
"start:js:prod": "cross-env NODE_ENV=production webpack-dev-server -w --progress",
"start": "parallelshell \"npm run reskindex:watch\" \"npm run start:res\" \"npm run start:js\"",
"start:prod": "parallelshell \"npm run reskindex:watch\" \"npm run start:res\" \"npm run start:js:prod\"",
Expand All @@ -58,7 +58,7 @@
"babel-runtime": "^6.11.6",
"bluebird": "^3.5.0",
"browser-request": "^0.3.3",
"extract-text-webpack-plugin": "^0.9.1",
"extract-text-webpack-plugin": "^3.0.0",
"favico.js": "^0.3.10",
"matrix-js-sdk": "0.10.1",
"matrix-react-sdk": "0.12.2",
Expand All @@ -75,7 +75,7 @@
"babel-cli": "^6.5.2",
"babel-core": "^6.14.0",
"babel-eslint": "^6.1.0",
"babel-loader": "^6.2.5",
"babel-loader": "^7.1.4",
"babel-plugin-add-module-exports": "^0.2.1",
"babel-plugin-transform-async-to-bluebird": "^1.1.1",
"babel-plugin-transform-class-properties": "^6.16.0",
Expand All @@ -101,8 +101,7 @@
"eslint-plugin-react": "^7.4.0",
"expect": "^1.16.0",
"fs-extra": "^0.30.0",
"html-webpack-plugin": "^2.24.0",
"json-loader": "^0.5.3",
"html-webpack-plugin": "^2.30.1",
"karma": "^1.7.0",
"karma-chrome-launcher": "^0.2.3",
"karma-cli": "^0.1.2",
Expand All @@ -112,27 +111,27 @@
"karma-sourcemap-loader": "^0.3.7",
"karma-spec-reporter": "0.0.31",
"karma-summary-reporter": "^1.3.3",
"karma-webpack": "^1.7.0",
"karma-webpack": "^2.0.4",
"matrix-mock-request": "^1.2.0",
"matrix-react-test-utils": "^0.2.0",
"minimist": "^1.2.0",
"mkdirp": "^0.5.1",
"mocha": "^2.4.5",
"parallelshell": "^3.0.2",
"postcss-extend": "^1.0.5",
"postcss-import": "^9.0.0",
"postcss-loader": "^1.2.2",
"postcss-mixins": "^5.4.1",
"postcss-nested": "^1.0.0",
"postcss-scss": "^0.4.0",
"postcss-simple-vars": "^3.0.0",
"postcss-import": "^11.1.0",
"postcss-loader": "^2.1.4",
"postcss-mixins": "^6.2.0",
"postcss-nested": "^3.0.0",
"postcss-scss": "^1.0.5",
"postcss-simple-vars": "^4.1.0",
"postcss-strip-inline-comments": "^0.1.5",
"react-addons-perf": "^15.4.0",
"react-addons-test-utils": "^15.6.0",
"rimraf": "^2.4.3",
"source-map-loader": "^0.2.3",
"webpack": "^1.12.14",
"webpack-dev-server": "^1.16.2"
"webpack": "^3.5.2",
"webpack-dev-server": "^2.7.1"
},
"optionalDependencies": {
"olm": "https://matrix.org/packages/npm/olm/olm-2.2.1.tgz"
Expand Down
7 changes: 7 additions & 0 deletions src/vector/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
<section id="matrixchat" style="height: 100%;"></section>
<noscript>Sorry, Riot requires JavaScript to be enabled.</noscript> <!-- TODO: Translate this? -->
<% for (var i=0; i < htmlWebpackPlugin.files.js.length; i++) {
if (_.endsWith(htmlWebpackPlugin.files.js[i], 'olm.js')) {
var array = htmlWebpackPlugin.files.js;
htmlWebpackPlugin.files.js.unshift(htmlWebpackPlugin.files.js[i]);
htmlWebpackPlugin.files.js.splice(i, 1);
}
}
for (var i=0; i < htmlWebpackPlugin.files.js.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. You're putting olm first? Why?
  2. The array variable is unused.
  3. Indents.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Because in webpack 3 they get randomly ordered which causes olm to sometimes not load. The old script relied on the case that the array is always ordered the same way.

2 and 3 can I fix after the weekend

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbkr I am not sure about 3. Should I do 2 spaces or 4? the html uses 2 spaces. Or do you mean the if that is missing one space?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok. Yeah, I can see how that would break. Generally all the indents should be 4 spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I will leave the html as is but fix the Js code in there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I wonder if we should be using chunksSortMode for the ordering (although given the docs don't tell you anything about what the different modes do, perhaps your solution is better...)

// Not a particularly graceful way of not putting the indexeddb worker script
// into the main page
if (_.endsWith(htmlWebpackPlugin.files.js[i], 'indexeddb-worker.js')) {
Expand Down
27 changes: 18 additions & 9 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,38 @@ module.exports = {
"theme-status": "./res/themes/status/css/status.scss",
},
module: {
preLoaders: [
{ test: /\.js$/, loader: "source-map-loader" },
],
loaders: [
{ test: /\.json$/, loader: "json" },
{ test: /\.js$/, loader: "babel", include: path.resolve('./src') },
rules: [
{ enforce: 'pre', test: /\.js$/, use: "source-map-loader", exclude: /node_modules/, },
{ test: /\.js$/, use: "babel-loader", include: path.resolve(__dirname, 'src') },
{
test: /\.scss$/,

include: [
path.resolve(__dirname, 'res/themes/status/css/'),
path.resolve(__dirname, 'node_modules/matrix-react-sdk/res/themes/light/css/'),
path.resolve(__dirname, 'node_modules/matrix-react-sdk/res/themes/dark/css/'),
],
// 1. postcss-loader turns the SCSS into normal CSS.
// 2. css-raw-loader turns the CSS into a javascript module
// whose default export is a string containing the CSS.
// (css-raw-loader is similar to css-loader, but the latter
// would also drag in the imgs and fonts that our CSS refers to
// as webpack inputs.)
// 3. ExtractTextPlugin turns that string into a separate asset.
loader: ExtractTextPlugin.extract("css-raw-loader!postcss-loader?config=postcss.config.js"),
use: ExtractTextPlugin.extract({
use: [
"css-raw-loader",
"postcss-loader"
],
}),
},
{
// this works similarly to the scss case, without postcss.
test: /\.css$/,
loader: ExtractTextPlugin.extract("css-raw-loader"),
use: ExtractTextPlugin.extract({
use: "css-raw-loader"
}),
},

],
noParse: [
// for cross platform compatibility use [\\\/] as the path separator
Expand Down