-
Notifications
You must be signed in to change notification settings - Fork 495
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
feat(runtime-config): export applyEnv
and getEnv
from nitropack
#2404
base: main
Are you sure you want to change the base?
Conversation
Thanks for PR β€οΈ I think this needs to be more thorough thinking before exposing from nitro runtime utils. Diverging Therefore putting PR on draft state until have a chance to think more.
A shared (build time) util via |
As another alternative approach we can think of different methods of runtime config expansion (via a util that is not env) so we don't add over complexity to "environment" definition but also allow a runtime method of customizing runtime config (this would help to somehow replicate "app config" extendability feature that is going to be deprecated in Nitro v3). I love to hear your thoughts about this method. |
applyEnv
and getEnv
from nitropack
It would be nice not to hard-code the use of Are you thinking that we just rename these utils so they aren't environment-focused (ie. naming for clarity), so they can be used generically for expansion, either as build-time kit utils or within the runtime (first in env, and then later with new feature)? That sounds very doable - say the word and I'll refactor. If you think this isn't so straightforward, I think we can go ahead and inline the existing nitropack behaviour in Nuxt for v3.12 like we did for nuxt/test-utils, and then take the time to get it right here, and switch downstream to using a forward-consistent implementation when it ships in What do you think? |
Usage of Runtime config override order:
Linked issues nuxt/nuxt#24224 and nuxt/nuxt#26960 are really good initiatives if the idea to have the inlined (kit ovverides + user config) > runtime overrides > runtime env vars Not sure which one is your current priority for Nuxt but I think these are two separate features we can introduce in nitro and keep in Nuxt until the 2.10 release so we don't diverge either for nitro what I suggest:
|
Thanks for the clear explanation. Yes, what was in my mind was purely about build-time utilities. The change from (The kind of usage I'm imagining was normalising |
Splitting into subtopics:
I strongly belive in making a new source/definition of environment variables (as proposed with |
Thank you! One more use-case to mention for allowing non-process environment is to 'preview' the resolved config if environment variables were to be set. I'm very much not proposing using arbitrary overrides for the actual resolved Nitro runtime config, just trying to make the underlying logic reusable in different contexts so it doesn't have to be duplicated. |
I guess at least all common runtimes allow to modify environment variables by directly modifying their env object ( The benefit of keeping runtime-config behavior strictly based on one standard (**) source (that is what environment variable in the runtime context means) is, that environment variables and how runtime config implicitly works with them won't diverge in a project that uses both environment variables and runtime config, behavior is consistently same. It makes tests more reliable too and also build and runtime utils consistent. (*) I understand that today, for testing modules with different sets of runtime configs, we strictly need this and also updating global env looks more hacky but let's consider that 1) (**) Today, we have already multiple definitions of environment variables: Actual system environment variables (that itself yet has no unified standard to access), |
@danielroe Shall we close PR and later continue discussing about this perhaps after |
π Linked issue
β Type of change
π Description
This adds
applyEnv
andgetEnv
as exports, as well as allowing them to operate on a customenv
object instead of reading fromprocess.env
.This would be a useful feature for frameworks to provide more accurate
runtimeConfig
at the build time. For example, we already inline these utils in nuxt/test-utils#655 and would need to do the same for nuxt/nuxt#24224 and nuxt/nuxt#26960.Of course, that would be fine, but it would nicer to be able to simulate the runtime behaviour of Nitro with these utils, as suggested in nuxt/nuxt#24224 (comment).
I've included them in the core
nitropack
export but I would almost prefernitropack/utils
, perhaps, to allow space for other similar utilities?π Checklist