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

Extensions build command is HARDCODING the NODE_ENV environment variable to: production #17556

Open
3 tasks done
sandinosaso opened this issue Feb 16, 2023 · 8 comments
Open
3 tasks done

Comments

@sandinosaso
Copy link

Checklist

Describe the Bug

We build an extensions that get ENVs variable to DB connection based on the NODE_ENV variable so for example:
In test env we use SQLite, in local we use Mysql and in production another Mysql

The problem we are facing, is that when we are working either in test or locally we keep having the NODE_ENV variable with value production we found this PR that introduced this change: https://github.com/directus/directus/pull/7627/files

We where not sure why we were always getting production value for the NODE_ENV variable and the PR above is the why.

The solution could be to not Hardcode the value instead taking it from the ENV where it is being used

Screenshot 2023-02-16 at 16 04 55
Screenshot 2023-02-16 at 16 04 39

To Reproduce

Just create a hook and do a console.log('NODE_ENV', process.env.NODE_ENV)

Hosting Strategy

Self-Hosted (Docker Image)

@github-actions
Copy link

Linear: ENG-710

@paescuj
Copy link
Member

paescuj commented Feb 21, 2023

I can well imagine that the reason for the hard-coded value is that it provides a bit of extra security and it can be assumed that extensions will eventually almost always run in a productive node environment and thus be tested in this context right away. (Maybe @rijkvanzanten / @nickrum can confirm or correct this)
In light of this assumption, I don't consider this an issue but I imagine we could make it configurable as a build option.

@peauc
Copy link

peauc commented Mar 7, 2023

What happens if you build your extension in its un-minified form ?

@nickrum
Copy link
Member

nickrum commented Apr 11, 2023

This was mainly done for App extensions because some libraries expect bundlers to replace process.env.NODE_ENV, which is not an issue with API extensions. It should be reasonably safe to assume that NODE_ENV is properly set when running an instance of Directus, so we might as well just remove the code that replaces it for API extensions.

@paescuj
Copy link
Member

paescuj commented Apr 11, 2023

This was mainly done for App extensions because some libraries expect bundlers to replace process.env.NODE_ENV, which is not an issue with API extensions. It should be reasonably safe to assume that NODE_ENV is properly set when running an instance of Directus, so we might as well just remove the code that replaces it for API extensions.

I see! Thanks! So just to be sure, you're suggesting to remove it only for API extensions but keep it for App extensions?

@rijkvanzanten
Copy link
Member

rijkvanzanten commented Apr 11, 2023

@paescuj Lets try to remove it from both if we can 👍🏻 Hopefully there's another way for the end-user to add it themselves on the app extensions side if it's required :) (Bit hard to explain why it exists, and only for some extension types in that other case)

@nickrum
Copy link
Member

nickrum commented Apr 11, 2023

I see! Thanks! So just to be sure, you're suggesting to remove it only for API extensions but keep it for App extensions?

Exactly 👍

Hopefully there's another way for the end-user to add it themselves on the app extensions side if it's required

There is not because process.env is a Node-thing not supported by browsers. Keeping it without replacement will break extensions using libraries that rely on it (in fact, Vue uses it and recommends consuming bundlers to replace it). While those libraries shouldn't use it in the first place, there is not really much we can do about it. Vite replaces it, esbuild replaces it, so we should do it too 😁

@rijkvanzanten
Copy link
Member

Copy that! Ty :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

No branches or pull requests

5 participants