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

feat: update webpack 3 to 4(working) #2

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

Conversation

yumetodo
Copy link
Collaborator

Resolve #1

Copy link
Collaborator Author

@yumetodo yumetodo left a comment

Choose a reason for hiding this comment

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

まだ作業中

use: process.env.NODE_ENV == 'production' ?
'css-loader?minimize' : 'css-loader',
//TODO: process.env.NODE_ENV doesn't work
use: process.env.NODE_ENV == 'production'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Webpack4になってどうすることになったんだっけか

Copy link
Collaborator Author

@yumetodo yumetodo Oct 31, 2018

Choose a reason for hiding this comment

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

https://webpack.js.org/migrate/4/#mode

うーん、modeっていう変数あるのか・・・?

@yumetodo yumetodo self-assigned this Oct 31, 2018
@yumetodo yumetodo requested a review from akihikodaki October 31, 2018 19:07
@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 1, 2018

webpack4全然わからない。
image

target: 'node',書いてるのに

Module not found: Error: Can't resolve 'fs'

file-loaderの設定書いてるのに

You may need an appropriate loader to handle this file type.

Copy link
Contributor

@akihikodaki akihikodaki left a comment

Choose a reason for hiding this comment

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

  • 一つのPRでいろいろやっちゃだめ。
  • cssnanoはPostCSSで使ってください。cssnanoはそういう用途に設計されている。
  • いらない変更があったりするので修正するときにはamendとforce-pushして。

play/common.webpack.config.js Outdated Show resolved Hide resolved
play/common.webpack.config.js Outdated Show resolved Hide resolved
@akihikodaki
Copy link
Contributor

electron を次のドキュメントにあるように commonjs として指定して。 (そう、ここで指定されるべきは fs ではなく electron)
https://webpack.js.org/configuration/externals/

@akihikodaki
Copy link
Contributor

file-loader のエラーわからないわかる。

@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 1, 2018

image
image
全く改善しない。

@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 1, 2018

electron を次のドキュメントにあるように commonjs として指定して。 (そう、ここで指定されるべきは fs ではなく electron)

やったけど( f941fcb ) 効果なくね?

@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 1, 2018

amendとforce-push

最後にrebase -iでいい感じにするのでご安心を

@@ -20,6 +20,9 @@ const path = require('path');

module.exports = Object.assign({
entry: '.',
externals: {
electron: 'electron'
Copy link
Contributor

Choose a reason for hiding this comment

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

commonjs としてだ。それではglobal scopeの electron variable を参照してしまう。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

うん?言いたいことがわからん。今

const electron = require('electron');

としているんだから、globalにおけばいいんじゃないの(わかってない

Copy link
Contributor

Choose a reason for hiding this comment

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

そうじゃなくて、 require('electron') が単に electron として置き換えられてしまうということ。

Copy link
Collaborator Author

@yumetodo yumetodo Nov 1, 2018

Choose a reason for hiding this comment

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

diff --git a/play/main/webpack.config.js b/play/main/webpack.config.js
index 57d76ae..fd95b1e 100755
--- a/play/main/webpack.config.js
+++ b/play/main/webpack.config.js
@@ -21,7 +21,9 @@ const path = require('path');
 module.exports = Object.assign({
        entry: '.',
     externals: {
-        electron: 'electron'
+        electron: {
+            commonjs: 'electron'
+        }
     }
        output: {
                filename: '[name].js',

こういうこと?エラーはあいかわらすだけど

@yumetodo yumetodo mentioned this pull request Nov 4, 2018
@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 4, 2018

あーもーだめだ(AMD)、さっぱりわからん。お手上げ。

@yumetodo yumetodo removed their assignment Nov 4, 2018
@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 4, 2018

ちなみにmakeはもちろんぶっ壊れているので治す必要がある。

@yumetodo yumetodo added the help wanted Extra attention is needed label Nov 4, 2018
@celluloce
Copy link
Collaborator

webpackでkagucho2018/play/mainのbuildですが、maindirectryに移動してwebpackを実行させるとできました。(理由はわかりませんが、既存のmakefileでもcdでdirectry移動した後webpackを実行している)
image

@celluloce
Copy link
Collaborator

kagucho2018/play/renderですが、コード書き換える等しないとbuild通らなさそうです。
rewrite_renderbranchでむちむち書き換えてます。

Copy link
Contributor

@akihikodaki akihikodaki left a comment

Choose a reason for hiding this comment

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

今見たんだけど、コード自体は直せばいいとして、 extract-text-webpack-plugin がまだbetaだからどのみち4に上げるのは反対だ。betaが外れてからでいい (2018ではやらない)

@celluloce
Copy link
Collaborator

https://weblike-curtaincall.ssl-lolipop.jp/blog/?p=2094
extract-text-webpack-pluginはWebpack4で使えないのでmini-css-extract-pluginを使おうと言ってます。
実際どうかわかりませんがmini-css-extract-pluginを使ってみませう。

@yumetodo
Copy link
Collaborator Author

yumetodo commented Nov 7, 2018

そういえば
miraiken/web#99
ではmini-css-extract-plugin使うように書き換えてたわ。もっとも謎のエラーで動かんのだが

@celluloce
Copy link
Collaborator

celluloce commented Nov 7, 2018

image
苦戦してる。

image
何が悪いんだろう...

image
image
わっかんね。

@celluloce
Copy link
Collaborator

#4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants