Skip to content
This repository has been archived by the owner on Sep 11, 2018. It is now read-only.

upgrade webpack from 1 to 2 #1189

Closed
wants to merge 16 commits into from
Closed

upgrade webpack from 1 to 2 #1189

wants to merge 16 commits into from

Conversation

rauleite
Copy link

@rauleite rauleite commented Mar 9, 2017

@codecov-io
Copy link

codecov-io commented Mar 9, 2017

Codecov Report

Merging #1189 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1189   +/-   ##
=======================================
  Coverage   72.72%   72.72%           
=======================================
  Files          12       12           
  Lines          66       66           
=======================================
  Hits           48       48           
  Misses         18       18

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd16e16...dab9d97. Read the comment docs.

@nathan-charrois
Copy link

nathan-charrois commented Mar 16, 2017

Any thoughts on passing sassLoader options directly instead of through LoaderOptionsPlugin? The implementation in this PR will break style paths when using @import './style' in components/routes unless context is provided.

ie.

{
  test: /\.scss$/,
  use: [
    {
      loader: 'sass-loader',
      options: {
        sourceMap: true,
        includePaths: [
          project.paths.client('styles')
        ]
      }
    }
  ]
}

More info: webpack-contrib/sass-loader#285 (comment)

@rauleite
Copy link
Author

Hi Nathan, I'm sorry I can not help you right now because of lack of time =/

I hope someone helps you!

@nathan-charrois
Copy link

No worries man, and thanks. I can confirm this is working, I'll make a separate PR if it doesn't make it into the webpack upgrade.

@rauleite
Copy link
Author

Very good! Good luck with this.

@Slumber86
Copy link

Slumber86 commented Mar 23, 2017

I think you forgot an option for sass-loader

options : {
        sourceMap : true,
        includePaths: [...project.paths.client('styles'), './node_modules',]
      } 

@Slumber86
Copy link

Also you missed to migrate from babel to webpack the ES2015 module resolution:
in project.config.js

  compiler_babel : {
    cacheDirectory : true,
    plugins        : ['transform-runtime'],
    presets        : [["es2015", { "modules": false }], 'react', 'stage-0']
  },

@collinwu
Copy link

Any status on this? I have an upcoming project and I'd love to leverage this repo, but with webpack 2 :)

@0-vortex
Copy link

Dedupe plugin is useless and prefix doesn't work for font loaders. I leveraged that with name: 'fonts/[name].[ext]', cheers !

@piu130
Copy link

piu130 commented Mar 29, 2017

Why don't we switch to neutrino? It's much easier and you don't have to debug build stuff anymore. See #1177.

@dvdzkwsk dvdzkwsk mentioned this pull request Apr 5, 2017
@pluswhite
Copy link

It's works follow your codes, thx ;) @rauleite

@@ -23,6 +23,7 @@ const karmaConfig = {
browsers : ['PhantomJS'],
webpack : {
devtool : 'cheap-module-source-map',
entry: './tests/test-bundler.js',
Copy link

Choose a reason for hiding this comment

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

Should this be ./${project.dir_test}/test-bundler.js like on line 11 of this file

@prestonbernstein
Copy link
Contributor

What's the status on this?

webpackConfig.plugins.push(
new webpack.HotModuleReplacementPlugin(),
new webpack.NoErrorsPlugin()
new webpack.NoEmitOnErrorsPlugin()
)
} else if (__PROD__) {
debug('Enabling plugins for production (OccurenceOrder, Dedupe & UglifyJS).')
webpackConfig.plugins.push(
new webpack.optimize.OccurrenceOrderPlugin(),
new webpack.optimize.DedupePlugin(),
Copy link

Choose a reason for hiding this comment

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

})
]
)

// File loaders
/* eslint-disable */
Copy link
Contributor

@matgessel matgessel May 9, 2017

Choose a reason for hiding this comment

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

I think we can eliminate the eslint-disable; it was there for the compact loader definitions.

})
]
)

// File loaders
/* eslint-disable */
Copy link
Contributor

@matgessel matgessel May 9, 2017

Choose a reason for hiding this comment

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

Less boilerplate:

const FONT_EXTS = new Map([
  ['woff', 'application/font-woff'],
  ['woff2', 'application/font-woff2'],
  ['otf', 'font/opentype'],
  ['ttf', 'application/octet-stream'],
  ['eot', 'application/vnd.ms-fontobject'],
  ['svg', 'image/svg+xml'],
]);
for (let [extension, mimeType] of FONT_EXTS) {
  webpackConfig.module.rules.push({
    test    : new RegExp(`\\.${extension}(\\?.*)?$`),
    loader  : 'url-loader',
    options : {
      name     : 'fonts/[name].[ext]',
      limit    : '10000',
      mimetype : mimeType,
    },
  });
};
webpackConfig.module.rules.push(
  {
    test    : /\.(png|jpg|gif)$/,
    loader  : 'url-loader',
    options : {
      limit    : '8192',
    },
  },
);

Note addition of .gif
Also note that file-loader does not support limit, so I assumed url-loader was intended.
I haven't tested this code yet.

query : project.compiler_babel
}, {
test : /\.json$/,
loader : 'json'
loader : 'json-loader'
}]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can eliminate this. JSON loader is built-in in Webpack 2.

delete loader.loaders
webpackConfig.module.rules.filter(rule =>
rule.loader && /css/.test(rule.loader)
).forEach(loader => {
Copy link

Choose a reason for hiding this comment

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

isn't it a misspell here?

...
  ).forEach(rule => {
...

@dvdzkwsk
Copy link
Owner

Moving to #1243.

Thanks for your work, I'll try to cherry-pick some of your fixes over. I have private project that just recently bumped webpack to 2.0, so I'm currently porting all of those changes over here.

@dvdzkwsk dvdzkwsk closed this May 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.