Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cdoublev opened this issue Jan 29, 2021 · 5 comments
Open

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

cdoublev opened this issue Jan 29, 2021 · 5 comments
Labels

Comments

@cdoublev
Copy link
Contributor

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.

@robotlolita
Copy link
Member

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
Copy link
Contributor Author

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
Copy link

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
Copy link

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants