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

Added UMD compiled version to build process #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ node_modules
# Optional REPL history
.node_repl_history
lib

# Compiled UMD version for https://unpkg.com/
dist
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
"description": "Denormalizer for normalizr",
"main": "lib/index.js",
"files": [
"dist",
"lib"
Copy link
Owner

Choose a reason for hiding this comment

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

We can use only one built version 😊 Remove lib and set "main": "dist/index.js" above.

Copy link
Author

Choose a reason for hiding this comment

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

Why one version ? Build process is preparing two different versions on purpose.

This is setup exactly as for example normalizr (or other node libraries). Your current version (only transpiled from ES6 to ES5) will be in lib as always. So library users will include it, in their npm projects like always. They can compile their own bundle, minify, ...

But dist will contain special UMD version of the library, that can be used directly in a browser - for example allowing it in JSBin playground via https://unpkg.com/. Thats why the file in dist is denormalizer.min.js and not index.js.

Copy link
Owner

Choose a reason for hiding this comment

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

Why one version

An UMD build works for CommonJS, AMD and global var, so I wouldn't mind to include this dist even for the standard lib. Anyway if this is the common practice then i can live with it 😄

Copy link
Author

@dhlavaty dhlavaty Oct 27, 2016

Choose a reason for hiding this comment

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

It is quite common. Check React, Redux or normalizr source codes.

https://github.com/reactjs/redux#installation

],
"scripts": {
"cover": "babel-node ./node_modules/istanbul/lib/cli cover -- _mocha --recursive --reporter spec",
"prebuild": "rimraf dist lib",
"build": "babel src --out-dir lib",
"build": "webpack && babel src --out-dir lib",
Copy link
Owner

Choose a reason for hiding this comment

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

We can build everything with webpack, i.e. skip the babel command here...

Copy link
Author

Choose a reason for hiding this comment

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

Don't think so. See ma answer #26 (comment)

"test": "mocha --compilers js:babel-core/register --recursive",
"test:watch": "npm run test -- --watch",
"prepublish": "npm run build",
Expand All @@ -33,7 +34,9 @@
"babel-cli": "^6.16.0",
"babel-core": "^6.17.0",
"babel-eslint": "^7.0.0",
"babel-loader": "^6.2.5",
"babel-preset-es2015": "^6.16.0",
"babel-preset-stage-1": "^6.16.0",
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we need stage-1 at all – is there a reason of why you added it?

"chai": "^3.5.0",
"chai-immutable": "^1.5.4",
"coveralls": "^2.11.14",
Expand All @@ -45,7 +48,8 @@
"immutable": "^3.8.1",
"istanbul": "^1.1.0-alpha.1",
"mocha": "^3.1.0",
"rimraf": "^2.5.4"
"rimraf": "^2.5.4",
"webpack": "^1.13.2"
},
"peerDependencies": {
"normalizr": "^2.0.0"
Expand Down
35 changes: 35 additions & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
var webpack = require('webpack');

module.exports = {
entry: './src/index',
module: {
loaders: [
{
test: /\.js$/,
loader: 'babel',
exclude: /node_modules/,
query: {
Copy link
Owner

Choose a reason for hiding this comment

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

@dhlavaty babel-loader uses .babelrc when available – it should be safe to remove the query object here, so we don't need to update two files when the babel config changes.

presets: ['es2015', 'stage-1']
}
}
]
},
output: {
filename: 'dist/denormalizr.min.js',
Copy link
Owner

Choose a reason for hiding this comment

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

This should be named index.js, not denormalizer.min.js

Copy link
Author

Choose a reason for hiding this comment

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

See my answer #26 (comment)

libraryTarget: 'umd',
library: 'denormalizr'
},
plugins: [
new webpack.optimize.OccurenceOrderPlugin(),
new webpack.DefinePlugin({
'process.env': {
'NODE_ENV': JSON.stringify('production')
}
}),
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think OccurenceOrderPlugin and setting the NODE_ENV to production is required – or am I missing something? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

My answer #26 (comment)

Copy link
Owner

Choose a reason for hiding this comment

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

The comment doesn't answer the question :) I'm curious why do we need OccurenceOrderPlugin and the NODE_ENV set to production here. OccurenceOrderPlugin ok, no much benefits using it, however the source doesn't include any particular conditions for production.

Copy link
Author

Choose a reason for hiding this comment

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

We want to have precompiled production and development UMD builds in the dist folder. You are probably right with OccurenceOrderPlugin, I was just copying webpack.config from different very simple setup. Do you want me to remove it ?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah I'd remove both the plugins, thanks!

new webpack.optimize.UglifyJsPlugin({
compressor: {
warnings: false
}
})
]
};