search

sindresorhus / is-observable

Check if a value is an Observable
MIT License
39 stars 7 forks source link

Add module version #6

Closed appsforartists closed 6 years ago

appsforartists commented 6 years ago

Igor Minar discusses some common reasons for bundles to be needlessly large in his ReactiveConf 2017 talk. One common problem is having commonJS in your dependency tree. Given the choice between adding a commonJS dependency or copy-pasting/reimplementing a 1-line function, I choose the latter. I know I'm not alone.

This PR adds an equivalent implementation as a module, which will allow tools to better statically analyze this dependency and minimize the amount of bloat when it's included in a bundle.

It has rebased been rebased from #5.

appsforartists commented 6 years ago

Just rebased from HEAD

sindresorhus commented 6 years ago

Sorry, I'm not interested in adding lots of boilerplate for this. It's totally possible to statically analyze CommonJS as long as you're not doing something weird. Here we just have a module.exports. Bloat usually comes from Node.js built-in polyfills rather than depending on CommonJS dependencies. There's a lot of misinformation in this area.

sindresorhus commented 6 years ago

Given the choice between adding a commonJS dependency or copy-pasting/reimplementing a 1-line function, I choose the latter. I know I'm not alone.

That's your choice, but you're going to have to do a lot of copy-paste.

appsforartists commented 6 years ago

That's too bad - it's just some package.json metadata and a second pass through the test suite - the actual logic of the function is the same.