Skip to content
This repository was archived by the owner on Jun 3, 2021. It is now read-only.

Upgrade to webpack 4.#111

Open
pattiereaves wants to merge 27 commits into
mainfrom
webpack-4
Open

Upgrade to webpack 4.#111
pattiereaves wants to merge 27 commits into
mainfrom
webpack-4

Conversation

@pattiereaves

Copy link
Copy Markdown

No description provided.

@pattiereaves pattiereaves requested a review from ostowe May 19, 2018 00:05
Comment thread .babelrc
"node": "6.11.4",
}
},
"modules": "commonjs",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

for some reason, this is the only way I could get the tests to work?! I don't understand why it wasn't necessary for the cli.

Comment thread CONTRIBUTING.md
At this point contribution guidelines are limited. This document contains information on setting up and developing locally; otherwise the primary concern for contributors is whether or not you are a member of the Alley Interactive organization (Huron's sponsor).

## Development
There are two primary comonents to Huron: the CLI and the web (browser) script. Since each of these is used for different purposes, there is a different build pipeline for each.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

All changes in this file are correcting typos

Comment thread package.json Outdated
"handlebars": "^4.0.11",
"handlebars-loader": "^1.7.0",
"html-loader": "^0.5.5",
"html-webpack-plugin": "jantimon/html-webpack-plugin#webpack-4",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@goodguyry goodguyry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The inconsistency in console statements' formatting (single-line vs. multi-line) bothers me, but it's obviously non-blocking. Sweet update! 🥇

Comment thread .eslintrc
"plugins": [
"jest"
],
// Do NOT change these rules

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😂🙅🚫💅

Comment thread config/webpack.browser.config.js Outdated
},
{
test: /\.js$/,
// exclude: /node_modules/,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 We don't need to exclude node_modules for either of these? If not, I'd say I'd say kill this comment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

we do, this was me debugging, I'm adding it back, good catch 😅

Comment thread src/cli/actions.js Outdated
chalk.red(`Could not delete: ${file.name}`)
);
console.warn(
chalk.red(`Could not delete: ${file.name}`));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this still be broken out in a separate line?

Comment thread src/web/index.js

if (prototype.length) {
id = prototype[0];
[id] = prototype;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔥🎸

Comment thread src/cli/server.js Outdated
info.errors
)
);
console.error(chalk.red(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here too

Comment thread src/cli/parseArgs.js
// Requires
/** @global */

// @todo this may be deprecated with webpack 4

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There still are some places where we are parsing for env.huron so it's not completely but we probably should look at a more webpack 4 way of approaching this.

Comment thread src/cli/server.js
'Webpack encountered warnings during compile: ', info.warnings
)
);
console.error(chalk.yellow('Webpack encountered warnings during compile: ', info.warnings)); // eslint-disable-line max-len

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i blame the ide

Pattie Reaves and others added 3 commits June 6, 2018 10:10
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.

3 participants