origamitower / folktale

[not actively maintained!] A standard library for functional programming in JavaScript
https://folktale.origamitower.com/
MIT License
2.05k stars 102 forks source link

process.env.FOLKTALE_DOCS is not replaced by Webpack 5 #229

Open cdoublev opened 3 years ago

cdoublev commented 3 years ago

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

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.

robotlolita commented 3 years ago

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.

cdoublev commented 3 years ago

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/

WidgetKing commented 3 years ago

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 commented 3 years ago

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
})]
robotlolita commented 3 years ago

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.