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

upgrade to webpack 2 #53

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

upgrade to webpack 2 #53

wants to merge 2 commits into from

Conversation

jiwhiz
Copy link

@jiwhiz jiwhiz commented Mar 5, 2017

No description provided.

@jiwhiz jiwhiz mentioned this pull request Mar 6, 2017
package.json Outdated
"webpack-merge": "^2.4.0"
"webpack": "^2.2.1",
"webpack-dev-server": "^2.4.1",
"webpack-merge": "^3.0.0"

Choose a reason for hiding this comment

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

About 2 hours ago 4.0.0 of webpack-merge came out ^^

.gitignore Outdated
@@ -2,3 +2,12 @@ elm-stuff/
node_modules/
tmp/
dist/

#IDE configuration files

Choose a reason for hiding this comment

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

I'm not a huge fan of IDE and OS specific stuff in project's .gitignore files. Imo this sort of things should be handled in a global .gitignore per machine. But after all that's just personal taste, so definitely up for discussion.

Copy link
Author

Choose a reason for hiding this comment

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

agree :) It is better to enable global .gitignore with command git config --global core.excludesfile ~/.gitignore_global.

@optikfluffel
Copy link

Apart from that: looks good to me 😀

Copy link

@ahstro ahstro left a comment

Choose a reason for hiding this comment

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

I'm no expert though, so double check everything I say :)

{
test: /\.(eot|ttf|woff|woff2|svg)$/,
loader: 'file-loader'
use: 'file-loader'
Copy link

Choose a reason for hiding this comment

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

I think this should still be loader. Check the // Do not use "use" here comment in the code block at https://webpack.js.org/guides/migrating/#module-loaders-is-now-module-rules

{
test: /\.elm$/,
exclude: [/elm-stuff/, /node_modules/],
loader: 'elm-webpack'
use: 'elm-webpack-loader'
Copy link

Choose a reason for hiding this comment

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

@@ -125,10 +128,10 @@ if ( TARGET_ENV === 'production' ) {
},
]),

new webpack.optimize.OccurenceOrderPlugin(),
new webpack.optimize.OccurrenceOrderPlugin(),
Copy link

Choose a reason for hiding this comment

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

This is on by default and can therefore be removed


// extract CSS into a separate file
new ExtractTextPlugin( 'static/css/[name]-[hash].css', { allChunks: true } ),
new ExtractTextPlugin( 'static/css/[name]-[hash].css'),
Copy link

Choose a reason for hiding this comment

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

I'm not sure if plain filename is supported, nor why you removed allChunks: true, but according to this, the new code should be

new ExtractTextPlugin({
  filename: 'static/css/[name]-[hash].css',
  allChunks: true
})

{
test: /\.elm$/,
exclude: [/elm-stuff/, /node_modules/],
loader: 'elm-hot!elm-webpack?verbose=true&warn=true&debug=true'
loader: 'elm-hot-loader!elm-webpack-loader?verbose=true&warn=true&debug=true'
Copy link

Choose a reason for hiding this comment

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

Should probably use the new "array of object"-syntax

use: [
  {
    loader: 'elm-hot-loader'
  },
  {
    loader: 'elm-webpack-loader',
    options: {
      verbose: true,
      debug: true,
      warn: true,
    }
  }
]

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.

3 participants