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

Reconfigure webpack for pollyfilling and tree shaking #37

Merged
merged 5 commits into from Oct 28, 2020
Merged

Reconfigure webpack for pollyfilling and tree shaking #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3696f10
Reconfigure webpack for pollyfilling and tree shaking
skarahoda Mar 17, 2020
2f2720c
Merge branch 'master' into feature/polyfill-and-tree-shake
skarahoda Oct 20, 2020
a92b811
Fix webpack config for tree shaking
skarahoda Oct 20, 2020
5d437b3
Apply suggestions from code review
skarahoda Oct 28, 2020
4a39b3e
Address pr review
skarahoda Oct 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@
"chalk": "^2.3.1",
"ci-info": "^2.0.0",
"commander": "^2.14.1",
"core-js": "^3.6.4",
"eslint": "^4.18.1",
"eslint-loader": "^2.0.0",
"fs-extra": "^5.0.0",
Expand Down
1 change: 1 addition & 0 deletions src/commands/create.js
View file
Open in desktop
Expand Up @@ -78,6 +78,7 @@ const createPackageJson = (root, { packageName, description, displayName, platfo
name: packageName,
version: "0.1.0",
description,
sideEffects: false,
scripts: {
start: "zem start",
build: "zem build",
Expand Down
2 changes: 1 addition & 1 deletion src/commands/test.js
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ module.exports = function (args) {
{
presets: [
[
require.resolve("@babel/preset-env"),
"@babel/preset-env",
{
targets: {
node: "current"
Expand Down
25 changes: 16 additions & 9 deletions src/config/webpack.common.js
View file
Open in desktop
Expand Up @@ -17,22 +17,29 @@ const copies = {
const { eslintConfig, main: entryPoint } = require(resolveExtensionPath("package.json"));
const eslintEnabled = eslintConfig || fs.readdirSync(extensionPath).find(f => f.startsWith(".eslintrc"));
const jsLoaders = [{
loader: require.resolve("babel-loader"),
Copy link
Member

Choose a reason for hiding this comment

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

If I remember it right, this helps when you run zem locally within its own directory.

How is your local development workflow? If we have a better way to run/test zem locally we can get rid of this.

☝️ We have another require.resolve usage in this module.

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 found several places that we use require.resolve and fix it. As far as I understand require.resolve import the module and return the file path instead of module itself. In our case this is redundant, since webpack can resolve the module from file name.

We also use it at manifest-builder: delete require.cache[require.resolve(pkgInfoPath)];. I'm not sure that we should remove from here also. That's why I didn't remove it.

loader: "babel-loader",
options: {
presets: [
[require.resolve("@babel/preset-env"), {
targets: {
chrome: 45,
safari: "9.1",
firefox: 45
[
"@babel/preset-env",
{
useBuiltIns: "usage",
corejs: 3,
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add core-js@3 to the list of "dependencies" of zem since this is going to be needed in case a feature that requires polyfill is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already added. "core-js": "^3.6.4" 🤔

modules: false, // Should be false to run tree shaking. See: https://webpack.js.org/guides/tree-shaking/
targets: {
chrome: 62,
safari: 11,
firefox: 59,
edge: 15
}
}
}]
]
]
}
}];

if (eslintEnabled) {
jsLoaders.push(require.resolve("eslint-loader"));
jsLoaders.push("eslint-loader");
}

module.exports = {
Expand All @@ -59,4 +66,4 @@ module.exports = {
}),
new ManifestBuilder(extensionPath, bundleName)
]
};
};
4 changes: 1 addition & 3 deletions src/config/webpack.prod.js
View file
Open in desktop
Expand Up @@ -2,10 +2,8 @@ const merge = require("webpack-merge");
const common = require("./webpack.common");

module.exports = merge(common, {
mode: "production",
output: {
filename: "[name].[chunkhash:8].js"
},
optimization: {
minimize: true
}
});
2 changes: 1 addition & 1 deletion src/template/src/fixtures/index.js
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@ import componentsData from "./components.json";
import versionData from "./version.json";
import { zeplin } from "../../package.json";

const defaultOptions = zeplin.options.reduce((options, option) => {
const defaultOptions = zeplin.options?.reduce((options, option) => {
options[option.id] = option.default;
return options;
}, {});
Expand Down