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

Multiple build targets with gulp & webpack#33

Merged
brockallen merged 4 commits intoDuendeArchive:devfrom
maxmantz:dev
May 24, 2016
Merged

Multiple build targets with gulp & webpack#33
brockallen merged 4 commits intoDuendeArchive:devfrom
maxmantz:dev

Conversation

@maxmantz
Copy link
Copy Markdown
Contributor

@maxmantz maxmantz commented May 24, 2016

See issue #31

I've taken parts from @tonyeung and @ArturDorochowicz and used gulp together with webpack to make the build targets. The sourcemaps are in the unminified versions under lib & dist (at the end of the file). The minified versions don't have sourcemaps to keep the file size down.

Comment thread package.json
"name": "oidc-client",
"version": "1.0.0-beta.7",
"description": "OpenID Connect (OIDC) & OAuth2 client library",
"main": "lib/oidc-client.min.js",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main now points to the minified version.

@brockallen
Copy link
Copy Markdown
Contributor

Yea, looks good. That's a hell of a sourcemap...

anyway, thanks

@brockallen brockallen merged commit cf5b4be into DuendeArchive:dev May 24, 2016
Comment thread gulpfile.js
},
plugins: [
],
devtool: 'inline-source-map'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Come to think of it... why we do need the source map for the non-min version?

Copy link
Copy Markdown
Contributor Author

@maxmantz maxmantz May 25, 2016

Choose a reason for hiding this comment

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

When debugging the unminified files can be used to jump to the ES6 source of the library because of the sourcemaps. I also wonder whether the sourcemap files should also be minified. Having them unminified doesn't add anything useful...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I wasn't aware they could link all the way back to the ES6 source files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, I want to release this this morning if possible... so any last minute suggestions related to this before I pull the trigger?

Copy link
Copy Markdown
Contributor Author

@maxmantz maxmantz May 25, 2016

Choose a reason for hiding this comment

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

Yeah - you can try it out by putting a debugger statement somewhere in your code and hit the console during execution. The source code shown in your browser should be the ES6 source file instead of the transpiled/minified version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only suggestion I have is to use the uglifyPlugins instead of the empty array for the sourcemap build targets in gulpfile.js. The sourcemaps should still work even when the build gets minified, so there is really no reason not to minify it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, added them and now the non-min (for dist) went from 1.9MB to 3.1MB.

Copy link
Copy Markdown
Contributor Author

@maxmantz maxmantz May 25, 2016

Choose a reason for hiding this comment

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

That's strange to say the least - I expected the exact opposite. Maybe minification increases the size of the sourcemap but I'm not sure. Did it only increase for dist or also for lib?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, just released it. Many thanks for your help @maxmantz!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Glad I could help.

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.

2 participants