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

fix(nuxt): use test/dev as manifest buildId when appropriate #23512

Merged
merged 2 commits into from Oct 3, 2023

Conversation

danielroe
Copy link
Member

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

With new app manifest enabled, we can potentially have a source of unreliable test data in that we have a constantly changing buildId which is difficult to mock (see danielroe/nuxt-vitest#354).

This PR defaults the buildId (unless user overrides) to test when running in e2e/unit test environments.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stackblitz
Copy link

stackblitz bot commented Oct 3, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@@ -213,7 +213,7 @@ export async function initNitro (nuxt: Nuxt & { _nitro?: Nitro }) {
// Add app manifest handler and prerender configuration
if (nuxt.options.experimental.appManifest) {
// @ts-expect-error untyped nuxt property
const buildId = nuxt.options.appConfig.nuxt!.buildId ||= randomUUID()
const buildId = nuxt.options.appConfig.nuxt!.buildId ||= (nuxt.options.test ? 'test' : randomUUID())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't checked impl but is dev also considered?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean in dev (non-test) we should default to dev or something similar?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes (either this field is optional or if we can serve it dev, i think a predictable value could be nice).

@danielroe danielroe changed the title fix(nuxt): use test as manifest buildId when testing fix(nuxt): use test/dev as manifest buildId when appropriate Oct 3, 2023
@danielroe danielroe merged commit 861d49e into main Oct 3, 2023
34 checks passed
@danielroe danielroe deleted the fix/test-build-id branch October 3, 2023 09:58
@github-actions github-actions bot mentioned this pull request Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants