Vike 0.4.161 process.env.NODE_ENV hard convention is problematic

[marviobezerra]

Description

Vike 0.4.161 breaks whole NX monorepo. None of my projects works. Even the NX VSCode plugin stops working.

The error message is:

Inner Error: Error: [vike][Wrong Usage] The environment is set to be a development environment by process.env.NODE_ENV === "development" which is forbidden upon building, see https://vike.dev/NODE_ENV

Reproduction:
https://github.com/marviobezerra/vike-nx

pnpm i
pnpm nx run vikedemo:serve

Workaround:

Downgrade to vike@0.4.160

[brillout]

Is there a reason you don't follow what the error message tells you? Closing but I'll re-open depending on the conversation.

[marviobezerra]

I didn’t set anything. It comes straight from NX and as I said, it was working fine until the latest version.

Also, I checked the docs and it’s vague there is no reason, just a don’t do it. To be honest, I think it’s odd that you rely so heavily on an environment variable that you break the whole monorepo. Why do you need that?

[marviobezerra]

More over, the sample repo, is a brand new NX and Vike. No customization at all.

Also, the documentation says “ We recommend the following.” and that it would be a warning and not an error.

[brillout]

Indeed the documentation was outdated. I just updated and improved it. It answers your questions.

[marviobezerra]

No, it does not answer my questions. In my opinion it's a bug that was introduced and quite serious one. If I update Vike it breaks all my projects. Because of an environment variable! Why do you need it so badly that you throw an exception?

Your first reaction was to close my issue just to keep the stats of low bugs and not considerer what I said. I think it rude! I've been trying to support your project and convince others to use it, you may have something good here. But your replies need some polishing.

[brillout]

Tolerating a faulty setup is asking for trouble. I understand that, strictly speaking, it's a breaking change. But I deem it so important that I decided to make it an error upon a minor semver update.

You'll likely disagree now, but the way I see it is that you'd, in hindsight, thank Vike's decision to force you to fix your setup.

As stated in the docs, make sure you follow the convention. You'll save yourself a lot of trouble.

[marviobezerra]

It's not the first time that I see your decisions break my efforts to use Vike. Again, you may have something good here but your attitude is pushing me away. My setup is not broken, I created a repo to show you that. I'm sure that you didn't even look at it and blamed me.

First was the use of @ on path alias, now the NODE_ENV value, what is next? I'm afraid of using snack case in my code because it will break again.

We have 20+ projects in this mono repo, it goes from NextJs, Solid, Vite, Storybook, and more. Not a single one breaks NX, but Vike (the underdog) does. Even our NX CI/CD is broken after this update.

If you think you are doing the right thing, go on! I will do the same, and move to something reliable.

[brillout]

Did you try to follow the convention?

To be clear, I'm not against reverting to make it a warning again, but I'll only do it if I'm provided with a rationale.

[marviobezerra]

As I said before, I didn't set the value. NX may do it. Check the repo and you will see.

[brillout]

Well, that would be a Nx issue then. Please dig a bit more about what's going on Nx's side and report back. So far I don't think it's a Vike issue.

It's important that Vike should be able to be used with Nx.

[marviobezerra]

I already spend more time than I should trying to figure it out, reporting the issue properly but you seem sure about your decision. The same as you did with the @ path alias.

If you ask my opinion, I should be able to set any environment variable to wherever value I want. If Vike needs to know if it’s production or not it should ask it explicitly in form of a parameter or create something specific to Vike, like VIKE_ENV.

[brillout]

Again, as I already said in my previous reply, I'm open to make it a warning again.

As for path aliases, you seem to have missed my latest replies in the conversation: I already changed my mind on that.

Please dig into why your Nx setup is broken and report back. It isn't my job to dig into your broken setup.

And, to be clear, I will revert and make it a warning instead of an error if it turns out to be a blocker.

[brillout]

Further polished https://vike.dev/NODE_ENV. I also thought it through again, and I still believe that how Vike's handling this is the correct way.

[JonhyBegood]

I’m getting the same error message but in my case I’m using Sonarqube for static code analysis.

We don’t set environment variables anywhere in our code, however, what happens inside of Sonarqube is outside of our control. We use Sonarqube as SaaS for ISO certifications, so the configurations are very restricted.

We faced a similar issue with Vite, but to remove a warning, and solution was to set VITE_USER_NODE_ENV, kind of what was proposed above.

Check https://github.com/vitejs/vite/blob/76f30ae23b92f9af910ec02d98e2baaefa12141f/packages/vite/src/node/config.ts#L559

Is it possible to revert it back?

[brillout]

@JonhyBegood Can you elaborate on what happens? What operation is failing and what is the NODE_ENV value? The more I know the better.

Is it possible to revert it back?

I'd rather go for a new setting dangerouslySkipConvention.nodeEnv: boolean while still showing a warning. The more annoying it is the better: setting the right NODE_ENV value is crucial. I'd actually recommend raising the issue at your tooling's issue tracker.

Edit: I created a feature request for it: #1499.

[JonhyBegood]

In summary, the analysis report fails and it blocks our deployment based on the ISO certifications that we are contractually obliged to maintain. So that is a critical issue based on our business model.

These certifications also include code warnings. It's a very complex topic, bear with me. 

In summary, we have 3 to 7 releases to fix the warnings. How many releases depending on the Sonarqube warning level. So, a warning now will eventually escalate to a critical issue and block our deployment.

Our certifications also check package versions. It defines if we must  update based https://semver.org. I'm mentioning that because right now, we downgraded Vike to unblock our deploy, but we will need to update it soon. 

We opened a Sonarqube ticket when we had issues with Vite and their answer was "Fix Vite, by creating a fork, or choose another bundling tool".

They hold the power and it sucks but we need to deliver.

[brillout]

I see. One potential issue is that, with the current plan I've in mind, you'll always get a warning.

Last question, so that I don't implement someting that doesn't work out (for you), what's the exact error message you get and its stack trace? What CLI command are you running to get the error?

[JonhyBegood]

There is no cli command as we set the project type Sonarqube does the rest.

I appreciate your work and help. However, I’m sorry to inform that after some considerations, the team and I decided to move to Remix. For now it’s a new and small project and this migration won’t after to much, hopefully it will grow and it could get complicated later.

Thanks for the amazing job to the open source community.

[brillout]

No worries and thanks for circling back on this.

If someone else is having issues with the strict NODE_ENV convention, leave a comment here.

[brillout]

@JonhyBegood FYI some of our sponsors have specific requirements that Vike was able to fulfill because Vike has been designed with flexibility in mind as well as because we were prioritizing their blockers (you get increased support & priority when sponsoring). Given that you have very specific requirements, such collaboration may work for you as well. Feel free to PM me on Discord (or email me) if that's something that peaks your interest.

[brillout]

The errors are now warnings. Released in 0.4.164. The plan is to make them errors again while enabling users to whitelist NODE_ENV values with a new setting allowNodeEnv.