search

google / incremental-dom

An in-place DOM diffing library
http://google.github.io/incremental-dom/
Apache License 2.0
3.54k stars 180 forks source link

Is dependency on Node.js core needed? #310

Closed thepian closed 7 years ago

thepian commented 7 years ago

It seems a bit strange to see references to process.env.NODE_ENV for a library that seems isomorphic. I'm trying to use SkateJS which depends on incremental-dom and build with RollupJS. This hopefully works fine, but references to process obviously blows up.

P.S. I did work around it by configuring an inject plugin for process, but should that be needed?

mashedcode commented 7 years ago

IMHO using process.env.NODE_ENV !== 'production' is pretty great and straight forward since no code modifications are necessary for testing and everyone knows what's meant by it right away because it's so popular. Use in the browser is covered by gulpfile.js babel transform-inline-environment-variables. I guess you simply want run this through the build system at first.

sparhami commented 7 years ago

P.S. I did work around it by configuring an inject plugin for process, but should that be needed?

If you want to build from source, you need to define it. There are also prebuilt UMD bundles (incremental-dom.js and incremental-dom-min.js) that remove the check and just have/don't have asserts enabled.

Expanding on @mashedcode's point, it allows you to integrate it into your overall build process and centrally flip assertions on/off for all the libraries you need with a single build config. The alternative would be to have a separate debug global that you could define in your build. Since most other libraries use process.env.NODE_ENV, I opted to follow the rest of the ecosystem.

thepian commented 7 years ago

I will close the issue as it is something you can configure with all build chains I know of. However I wish there was some future option in ES6, macros perhaps, to resolve this at build time.