-
Notifications
You must be signed in to change notification settings - Fork 102
feat(e2e): support env
option for startServer
#640
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
Merged
danielroe
merged 12 commits into
nuxt:main
from
BobbieGoede:feat/module-util-set-runtime-config
Jan 17, 2024
Merged
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ae80876
feat: add `setRuntimeConfig` module utility
BobbieGoede b7355dc
test: confirm `setRuntimeConfig` works
BobbieGoede 4dd3e8e
Merge remote-tracking branch 'origin/main' into feat/module-util-set-…
danielroe fe1c8d3
refactor: move to e2e utils
danielroe cc5305d
chore: remove space
danielroe ca54fc3
refactor: avoid exposing other utils
danielroe 0e60b70
fix: move `scule` to dependency
danielroe 46df850
fix: `setRuntimeConfig` not passing env in options object
BobbieGoede 8bf8cc7
feat: add optional `envPrefix` argument to `setRuntimeConfig`
BobbieGoede 7985b70
refactor: move solely as option to `startServer`
danielroe 28d8bc5
Merge remote-tracking branch 'origin/main' into feat/module-util-set-…
danielroe 4ad32a6
chore: add name to module package.json
danielroe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
<template> | ||
<div>basic</div> | ||
<div>basic <span>{{ config.public.myValue }}</span></div> | ||
</template> | ||
|
||
<script setup> | ||
const config = useRuntimeConfig(); | ||
</script> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import { snakeCase } from 'scule' | ||
import { startServer } from './server' | ||
|
||
export function flattenObject(obj: Record<string, unknown> = {}) { | ||
const flattened: Record<string, unknown> = {} | ||
|
||
for (const key in obj) { | ||
if (!(key in obj)) continue | ||
|
||
const entry = obj[key] | ||
if (typeof entry !== 'object' || entry == null) { | ||
flattened[key] = obj[key] | ||
continue | ||
} | ||
const flatObject = flattenObject(entry as Record<string, unknown>) | ||
|
||
for (const x in flatObject) { | ||
if (!(x in flatObject)) continue | ||
|
||
flattened[key + '_' + x] = flatObject[x] | ||
} | ||
} | ||
|
||
return flattened | ||
} | ||
|
||
export function convertObjectToConfig(obj: Record<string, unknown>, envPrefix: string) { | ||
const makeEnvKey = (str: string) => `${envPrefix}${snakeCase(str).toUpperCase()}` | ||
|
||
const env: Record<string, unknown> = {} | ||
const flattened = flattenObject(obj) | ||
for (const key in flattened) { | ||
env[makeEnvKey(key)] = flattened[key] | ||
} | ||
|
||
return env | ||
} | ||
|
||
export async function setRuntimeConfig(config: Record<string, unknown>, envPrefix = 'NUXT_') { | ||
const env = convertObjectToConfig(config, envPrefix) | ||
await startServer({ env }) | ||
|
||
// restore | ||
return async () => startServer() | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not too much fan of this API.
setRuntimeConfig
sounds like doing a runtime operation without full server restart and nitrouseRuntimeConfig
- when used correctly in context - reflects any runtime env changes as soon as they change without need of a manual server restart. If manual server restart is desired for any incompatibility, it should be opt-in IMO with something likesetRuntimeConfig({}, { restart: true }
(we could also automatically do it for well known keys such as baseurl)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the issue that it sounds too similar to
useRuntimeConfig
while not functioning in a similar way (trigger restart, not reactive), or that the server gets restarted either way?The reason for restarting the server is that I don't know how (or if it is possible) to update the env variables of the running server process. It would be preferable to be able to handle/apply this without a server restart, if that is not possible I don't think opt-in restart makes much sense in my opinion.
I agree that it sounds similar to
useRuntimeConfig
, maybe a better name could beapplyRuntimeConfig
,overrideRuntimeConfig
oroverwriteRuntimeConfig
?Something else I considered would be something like this:
So that restoring/resetting config always happens without additional action, I think having to restore it separately can easily be forgotten.
Either way, I'm open to suggestions 😅 I'm not aware of other ways to easily test runtime config support for module authors, so I think this is essential if we want more modules to reliably support it.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An IPC signal can do it using process.send . Alternatively, we can export a dev middleware when starting nuxt in test mode to allow it. It is much cheaper than full-reload and also allows stateful testing when we need to see the effect of one even without for example losing connection. Please ping me if need any more insights on how to do this.
If you are thinking this (as a set-and-restart) would be still a useful tool and faster to iterate, I would suggest to explicitly call it
startServerWithRuntimeConfig
as it does this exactly.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hint!
I managed to communicate with the server process using
process.send
and am able to make changes to theprocess.env
while keeping it running. Using a server route I can confirm thatuseRuntimeConfig(event)
contains the updated values, unfortunately it doesn't seem to update in plugins (testing server side) before/after page reload.Does overwriting runtime config variables with environment variables only work in server routes or should this also work elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm on the wrong track entirely but it seems like things get a bit more complicated.. 😅 Please let me know if this sounds right or not.
1. Server routes
Runtime config can be overwritten by setting environment variables using an IPC signal.
2. Server-side plugins and such
Runtime config is frozen, overwriting requires restarting server with environment variables.
3. Client-side
Runtime config is reactive, can be changed using
page.evaluate
or something along those lines.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to (also) listen via a Nuxt plugin but seems so strange because last I recall Nuxt also uses direct import from Nitro utils...
Do you mind to push your latest changes? If feel too much different another (draft PR) might also be good so I can check locally somehow 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been working on this locally inside the
@nuxtjs/i18n
repo as I already have tests for runtime config there, also I'm not sure what the intended workflow in this repo is but I find myself having to rundev:prepare
andprepack
before running tests/checking if the changed test utils work.The code is a bit of a mess as I was exploring what worked and what didn't, I'll clean it up a bit and open a draft PR.
My implementation uses a Nuxt plugin to start listening for messages, is that what you mean?
When an
H3Event
is passed touseRuntimeConfig
it does pick up the changes, otherwise it keeps using the original values. I think you describe the same issue of having to restart the server/Nuxt instance here: nuxt/nuxt#14330.I'll link the draft PR here once I have it ready!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you probably can try swapping/adding a Nitro plugin. It runs before and also applies for server logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pi0
Here's the draft PR #670