folktale icon indicating copy to clipboard operation
folktale copied to clipboard

process.env.FOLKTALE_DOCS is not replaced by Webpack 5

Open cdoublev opened this issue 4 years ago • 5 comments

By default Webpack 5 only replace process.env.NODE_ENV when bundling. But Folktale uses process.env.FOLKTALE_DOCS and process.env.FOLKTALE_ASSERTIONS in three distribution files. When executed, those bundled files throws an error because process is undefined.

Steps to reproduce

It can be reproduced by bundling with Webpack 5 a file that contains eg. Task.of().

Expected behaviour

Run the bundle without throwing an error.

Observed behaviour

The bundled file throws an error because process is undefined.

Environment

  • OS: Debian Buster
  • JavaScript VM: V8 (NodeJS 15)
  • Folktale version: 2.3.0

Additional information

It can be fixed by adding plugins: [new webpack.EnvironmentPlugin({ FOLKTALE_DOCS: false })] in the Webpack configuration file. This could be documented in the Folktale documentation.

Or FOLKTALE_DOCS and FOLKTALE_ASSERTIONS can be moved/nested as object properties in process.env.NODE_ENV, but I'm aware that browsers are not necessary the main platform target of this library, therefore this change might not be worth it. However, and to answer this comment (what most users of the library are interested in seeing), this change could lower the barrier of entry for adopting Folktale, which I should say is IMHO the best FP ADT library I found and use since a few years now.

cdoublev avatar Jan 29 '21 10:01 cdoublev

Ah, I'm not very familiar with JS tooling at this point. Is that a new thing in WebPack?

Looks like there aren't many instances of it in the source, so it should be doable to just test for process first. I was worried it was being used for dead code elimination, but that isn't the case. I'll try to push a new version this week or the next.

robotlolita avatar Jan 31 '21 15:01 robotlolita

Yes, Webpack 5 now only set process.env.NODE_ENV from configuration.nodeEnv = configuration.mode (default is 'production'). I also think testing for process will do the trick as well. Thank you! 👍

process is not defined.

[...]

Want to use environment variables with process.env.VARIABLE? You need to use the DefinePlugin or EnvironmentPlugin to define these variables in the configuration. Consider using VARIABLE instead and make sure to check typeof VARIABLE !== 'undefined' too. process.env is Node.js specific and should be avoided in frontend code.

https://webpack.js.org/migrate/5/

cdoublev avatar Jan 31 '21 16:01 cdoublev

I just ran across this one too. Because folktale was being used by a dependency and not the current project it was not all that easy to track down the issue.

Thanks for cdoublev for posting the plugins: [new webpack.EnvironmentPlugin({ FOLKTALE_DOCS: false })] fix. It worked for me.

But, yeah. I would be in favor of some kind of update which meant folktale didn't die instantly whenever you try to run it on the web.

WidgetKing avatar Apr 15 '21 08:04 WidgetKing

Spoke too soon. To get it all working I also had to add FOLKTALE_ASSERTIONS to the environment plugin:

plugins: [new webpack.EnvironmentPlugin({ 
   FOLKTALE_DOCS: false,
   FOLKTALE_ASSERTIONS: false
})]

WidgetKing avatar Apr 15 '21 08:04 WidgetKing

This fell through my todo list. The proper fix is simple, but releasing it is going to take a while as the build system has degraded quite a bit over the years.

robotlolita avatar Apr 15 '21 16:04 robotlolita