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.

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

Use rollup dist bundle instead. Add esm module distribute for rollup ecosystem. (#468) #628

Closed
wants to merge 2 commits into from

Conversation

allex
Copy link

@allex allex commented Oct 15, 2018

Use rollup dist bundle instead. Add dist/debug.esm.js module distribute for rollup ecosystem. (#468)

Use for Rollup ecosystem

import resolveId from '@allex/rollup-plugin-node-resolve'

// rollup.config.js
export default {
  input: 'src/main.js',
  output: {
    file: 'dist/bundle.js',
    format: 'umd'
  },
  plugins: [
    // ...
    resolveId({
      extensions: ['.js', '.ts'],
      alias: {
        'debug': 'debug/dist/debug.esm.js',
      }
    })
  ]
  // ...
}

@coveralls
Copy link

coveralls commented Oct 15, 2018

Coverage Status

Coverage increased (+0.8%) to 87.597% when pulling 8ce6624 on allex:dev into 5fadf45 on visionmedia:master.

@Qix-
Copy link
Member

Qix- commented Dec 13, 2018

Hi there - before I really comment on this at a high level, please make sure all of the formatting is consistent. This also looks like it adds a lot of superfluous stuff that doesn't need to be there. Could you make sure you're adding the minimal amount necessary to build with rollup?

Also, I'm still not sold on the idea of rolling up modules. That's been an area of extensive discussion over at chalk, too.

@Qix- Qix- added change-minor This proposes or provides a change that requires a minor release awaiting-response This issue or pull request is awaiting a user's response labels Dec 13, 2018
@allex
Copy link
Author

allex commented Dec 17, 2018

Hi there - before I really comment on this at a high level, please make sure all of the formatting is consistent. This also looks like it adds a lot of superfluous stuff that doesn't need to be there. Could you make sure you're adding the minimal amount necessary to build with rollup?

Also, I'm still not sold on the idea of rolling up modules. That's been an area of extensive discussion over at chalk, too.

Actually, for rollup ecosystem, we need bundle a out dist with ES module syntax. so i've dropped bundler of browserify and use rollup instead.

and the debug.d.ts for typescript ecosystem declaration file.

@Qix-
Copy link
Member

Qix- commented Dec 19, 2018

This is being closed - reasoning to follow. However, for the future:


At a high level, I don't see a benefit of this. Really. This is just adding to the build process and doubling the size of a package that garners millions of downloads.

My stance on the pre-babelification of modules is that of Sindre Sorhus's: sindresorhus/ama#446 (comment)

We already deprecated the build process in 4.x. Babel is ultimately going away. This is moving us backward at this point.

Furthermore, debug is not that complicated. It has pretty much no bootup cost and is already pretty cheap. Further when #612 is added, it will already add some size - so I want to minimize the extra cruft as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-response This issue or pull request is awaiting a user's response change-minor This proposes or provides a change that requires a minor release
Development

Successfully merging this pull request may close these issues.

None yet

3 participants