fix(core): require('debug') throws exception in browser

[MichaelRando]

Remove 'require()' call from debug.js because it breaks es builds in browser via rollup.js

Fixes it

[levithomason]

Hm, this will bundle the debug module in server side builds, production, and tests. The previous inline require is wrapped in a conditional:

https://github.com/Semantic-Org/Semantic-UI-React/pull/1237/files#diff-11b44325a1a4744c65365db1d18055fdR7

Which is removed with dead code elimination, so it isn't bundled server side, in production, nor tests. The current change will always import the module and bundle it in systems that do not make use of tree shaking.

We'll need to find a solution that does not bundle it in either case. Not sure on this one yet.

[codecov-io]

Current coverage is 95.89% (diff: 100%)

Merging #1237 into master will not change coverage

@@             master      #1237   diff @@
==========================================
  Files           880        880          
  Lines          4901       4901          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           4700       4700          
  Misses          201        201          
  Partials          0          0          

Powered by Codecov. Last update d3b1acd...b213262

[levithomason]

Could you better define "breaks it" for me here? I'd like to find a solution that works regardless of the build tool a user chooses.

[MichaelRando]

"breaks it" i.e. 'require('debug') throws exception in the browser'

This is an interruption of normal program execution. require(...) is not a part of es6, I believe it's a function from a runtime module loader that browsers do not natively support. The es6 build is for es6 modules, and the imports are unfortunately top-level and not determined at runtime, as that's how the language is these days...

The repro would be to use the module in an es6 browser environment without the webpack loader. Like I did with rollup.

[levithomason]

OK, I see the issue now, thanks. I'll have to find a solution to this that does not include the import statement for debug at all in production. Otherwise, when the package is built, it is going to include the node version of the debug module.

Goals are:

• do not include any require() calls in the es build
• do not bundle the debug module in production, regardless of build tool chosen

I have a couple ideas, others welcome. I'll come back to this one when I have time.

[layershifter]

I've performed some tests there. Rollup doesn't support System.import 😕 Only ES imports can be used, so we can't use conditionals there because 'import' and 'export' may only appear at the top level.

My proposal is move debug to separate package which will have ES version without conditional import and CJS version with require. Also, it's single place where we use require().

[levithomason]

Update on my thoughts here. We currently use the debug module more as in internal debugger. Even if users are using the debug logs, they are not currently available in production as the module will export a noop. Because of this, I'm considering simply stripping these statements from all builds.

We could use something like https://github.com/codemix/babel-plugin-trace for these logs instead. We could then entirely remove these statements automatically in our builds.

There is also https://github.com/GavinJoyce/babel-plugin-remove-functions which would allow us to target individual functions for removal. This also would enable us to keep the existing debug module and its usage intact during development. We'd simply not include the debug statements in the source code for any /dist build.

[layershifter]

Closing in favor of #1663.