redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.36k stars 1k forks source link

[Bug?]: Cannot use environment variable in redwood.toml for `sourceMap` option #10214

Open oliwheeler opened 8 months ago

oliwheeler commented 8 months ago

What's not working?

As described here, it's possible to use environment variables in redwood.toml.

However, since sourceMap is a boolean option, it casts the string "false" to boolean true. So sourceMap = "${SOURCE_MAP_ENABLED:false}" gets interpeted as true when SOURCE_MAP_ENABLED is unset.

How do we reproduce the bug?

  1. Set sourceMap = "${SOURCE_MAP_ENABLED:false}" in the redwood.toml file.
  2. The source maps will be in web/dist/assets

What's your environment? (If it applies)

No response

Are you interested in working on this?

Josh-Walker-GM commented 8 months ago

Hey @oliwheeler 👋

So our TOML config parsing happens here with these two functions: https://github.com/redwoodjs/redwood/blob/0b185043b1987599487bf1c5198ec47994db4c5d/packages/project-config/src/config.ts#L205-L229

You can see why this is causing the behaviour you didn't quite expect. We perform the env interpolation before we parse the toml. This means we'd do the following during our env interpolation stage:

sourceMap = "${SOURCE_MAP_ENABLED:false}" -> sourceMap = "false" -> sourceMap: "false"

Then when this is parsed from TOML it'll be a string "false", which is truthy and so we get the problem you raise here.

One workaround is to avoid passing an alternative option so: sourceMap = "${SOURCE_MAP_ENABLED:}" and then you'll end up with an empty string that is falsy.

Another workaround is to avoid quotation marks so: sourceMap = ${SOURCE_MAP_ENABLED:false} and you'll end up with only false and not "false" but before we interpolate values this isn't valid TOML markup. This will likely cause your IDE to complain with red squiggles.

I'm not sure what we could do to avoid this happening. We likely lack enough information to be able to do anything like transform "false" to false when we discover it in the TOML.

I'd be good to hear more about what you'd have expected and how we can make this less confusing or unexpected. Would a warning in our docs in that section you linked to have been helpful?

oliwheeler commented 8 months ago

Thank you for the explanation!

I just tried the first workaround but for some reason that still produces source maps.

The second workaround works but as you say, it's not valid so red squiggles.

Rather than having ENV VAR's within the config, is it in the cards to be able to supply the path to the TOML file as a CLI command parameter? Like yarn rw build --config redwood.production.toml?

Josh-Walker-GM commented 8 months ago

It's not on the cards at the moment but let me think the problem over a little bit more and have a talk with some of the team about it.

This isn't blocking you at the moment right?

oliwheeler commented 8 months ago

It's not on the cards at the moment but let me think the problem over a little bit more and have a talk with some of the team about it.

This isn't blocking you at the moment right?

Not blocking at all :)

Josh-Walker-GM commented 8 months ago

Hey @oliwheeler could you try: sourceMap = "${SOURCE_MAP_ENABLED: }" and confirm that it works for you? Taking care to note the space after the colon here. As far as I can tell if you leave the space then it will in fact end up as an empty string once it's interpolated. That will then be falsy and you'll get no source maps.

I confirmed that I was wrong with sourceMap = "${SOURCE_MAP_ENABLED:}" since that isn't valid syntax for the interpolator so you end up with source maps being produced as you say. My bad on that one.

This doesn't feel particularly ergonomic and I will bring it up with the team. However, I suspect we're unlikely to carve out some dedicated time ourselves to implement anything to make this work better in the near future just because of priorities.

oliwheeler commented 8 months ago

Thanks! The new solution with the space works :)