Skip to content

Remove webpack and add Rollup as a module bundler#772

Merged
luisrudge merged 3 commits intomasterfrom
feature-rollup-bundler
Jun 12, 2018
Merged

Remove webpack and add Rollup as a module bundler#772
luisrudge merged 3 commits intomasterfrom
feature-rollup-bundler

Conversation

@luisrudge
Copy link
Copy Markdown
Contributor

@luisrudge luisrudge commented Jun 8, 2018

Rollup is better suited to do module bundling for libraries. One of the reasons is because it allows us to easily target different types of bundles like UMD, ES and CJS. This PR only maintains the UMD bundle for now, but we can add ES modules later. The benefit of using ES modules later is that consumers of this library will be able to do "tree shaking", which means their bundle will only include what they use, everything else will be left off. This can greatly reduce the bundle size for people only using WebAuth and not Authentication, for example.

Here's a great article about the difference between webpack and rollup for libraries:
https://medium.com/webpack/webpack-and-rollup-the-same-but-different-a41ad427058c

Summary of changes:

  • Files will go to both the dist and build folder
  • Changed CDN to read files from the dist folder instead of build
  • Remove all gulp and webpack related packages
  • added browser field in package.json which specifies alternative files to load for people bundling for the browser
  • added files field in package.json so we deploy the distribution files to the npm registry (it will allow users to use services like https://unpkg.com/)

Before

image

After

image

@luisrudge luisrudge force-pushed the feature-rollup-bundler branch from 4b98683 to 6035e67 Compare June 8, 2018 12:58
@luisrudge luisrudge force-pushed the feature-rollup-bundler branch from 6035e67 to c26c61b Compare June 8, 2018 13:14
@joshcanhelp
Copy link
Copy Markdown
Contributor

joshcanhelp commented Jun 11, 2018

@luisrudge - Thank you for the detailed run-down! Informative screenshots and good supporting article:

If you need code-splitting, or you have lots of static assets, or you’re building something with lots of CommonJS dependencies, Webpack is a better choice. If your codebase is ES2015 modules and you’re making something to be used by other people, you probably want Rollup.

I'm definitely sold that this is the right way to go and I don't know how helpful an in-depth config file review would be. My main concern here, though, is what we talked about last week: the change from build to dist. I agree that dist is generally the "standard" directory, and the one I typically use myself, but is this change required for Rollup? Is it considered breaking for anyone? Should it be done in a separate PR?

In short: is there a case where changing this output folder will cause a break in someone's deployment procedure, even if it's in the minority of cases?

Edit: one more that came up from the article ... is there any reason to include a pkg.module declaration in package.json?

name: 'auth0',
file: 'dist/auth0.js',
format: 'umd',
sourcemap: isProduction ? false : 'inline',
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.

Why are we checking isProduction if these are dev files?

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.

isProduction is more like: areWeBuildingAProductionFile

commonjs(),
json(),
replace({
__DEV__: isProduction ? 'false' : 'true',
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.

! isProduction

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.

Not sure what that means 😬

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.

Simpler statement, same meaning

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.

nope, I need the result to be a string, so it'd have to be either (!isProduction).toString() or what I have there

rollup.config.js Outdated
input: 'plugins/cordova/index.js',
output: {
name: 'CordovaAuth0Plugin',
file: 'dist/cordova-auth0-plugin.min.js',
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.

Might be better to have dist/ as a var

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.

probably 😬

@luisrudge
Copy link
Copy Markdown
Contributor Author

I'm definitely sold that this is the right way to go and I don't know how helpful an in-depth config file review would be. My main concern here, though, is what we talked about last week: the change from build to dist. I agree that dist is generally the "standard" directory, and the one I typically use myself, but is this change required for Rollup? Is it considered breaking for anyone? Should it be done in a separate PR?
In short: is there a case where changing this output folder will cause a break in someone's deployment procedure, even if it's in the minority of cases?

Yeah, I'll change to use build and dist.

Edit: one more that came up from the article ... is there any reason to include a pkg.module declaration in package.json?
Once we have ES modules fully supported, then yes

@luisrudge luisrudge force-pushed the feature-rollup-bundler branch from f009951 to 9c1e2bd Compare June 12, 2018 16:24
Copy link
Copy Markdown
Contributor

@joshcanhelp joshcanhelp left a comment

Choose a reason for hiding this comment

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

LGTM

@luisrudge luisrudge merged commit b00bd41 into master Jun 12, 2018
@luisrudge luisrudge deleted the feature-rollup-bundler branch June 12, 2018 22:41
@luisrudge luisrudge mentioned this pull request Jul 15, 2018
luisrudge added a commit to auth0/idtoken-verifier that referenced this pull request Feb 26, 2019
In the same approach as auth0/auth0.js#772 and auth0/auth0.js#774, this PR modernizes our build system.

- Uses https://github.com/developit/microbundle to build the project. This has the benefit of building every type of module (cjs, esm, umd) in one go with 0 config
- Migrate to ES Modules (we need this to proper implement three shaking)
- Migrate tests to ES Modules as well
- Fix telemetry file generation
- Fix docs files generation
- Prettify code in a precommit hook
- Check for ES5 compatible code to make sure we don't ship unsupported javascript syntax
- Print bundle size
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants