Skip to content

Conversation

@Harm-Nullix
Copy link

  • Refactor useState and clearNuxtState to improve statemanagement and reset functionality, introduce _state for internal use.

🔗 Linked issue

#32117

📚 Description

The setup for this PR is a starting point with a proposed solution
It does break the current logic as it does not automatically sets the state to undefined but resets it to the given init function

Other paths considered:

  1. make the init parameter a options object (fallback to old init param),
    • to define a default value alongside the init value
    • or to flag the state to reset / not reset to undefined
  2. flip the flag that is now given to clearNuxtState to keep current logic as is.
  3. Setting a object in the state.ts file that holds the init functions by key rather than using nuxtApp._state

  1. I did not want to do this because it makes the function of useState less clear
  2. The solution and logic way of thinking resetting to a default value instead of undefined seemed more in line with Nuxt composables like useAsyncData
  3. I did this because it is more in line with other data objects and maybe clears the path for further improvements for the state across the whole NuxtApp.

TODO

When a solid path is determined, I will add guiding docs to the current functionality

… management and reset functionality, introduce `_state` for internal use.
@bolt-new-by-stackblitz
Copy link

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

@coderabbitai
Copy link

coderabbitai bot commented Oct 20, 2025

Walkthrough

The changes refactor Nuxt's state management system by introducing a new internal metadata storage mechanism (_state on the Nuxt app instance) to track per-key state and default functions. The useState composable is updated to accept a UseStateOptions<T> type for initialization and returns a computed reference that proxies to this internal storage. The clearNuxtState function gains an optional reset parameter and delegates per-key clearing logic to a new clearNuxtStateByKey helper, which handles both payload removal and state reset to initial or undefined values. Comprehensive test coverage has been added to validate state lifecycle, reset semantics, and payload synchronisation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The changes introduce new initialisation logic and internal state storage mechanisms in state.ts with modified public API signatures. Whilst the structural changes are localised to a single primary file and the logic is reasonably focused, the interplay between metadata storage, hydration, and reset behaviour requires careful reasoning. The addition of a new helper function and the introduction of the _state field in nuxt.ts add moderate complexity. The test additions are largely homogeneous patterns testing reset and removal scenarios, which reduces overall review effort. The heterogeneous nature of the logic changes (new storage pattern, computed reference behaviour, reset semantics) necessitates separate reasoning for key sections.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "feat(nuxt): add reset to default functionality for useState and clearNuxtState" is directly and clearly related to the main changes in the changeset. The summary confirms that the core modifications involve refactoring these two composables to implement reset-to-default functionality rather than resetting to undefined, introducing an internal _state field to manage this behaviour, and updating method signatures to support the new reset parameter. The title is concise, specific, and accurately reflects the primary change from the developer's perspective without noise or vague terminology.
Description Check ✅ Passed The PR description is meaningfully related to the changeset. It explicitly discusses the refactoring of useState and clearNuxtState, the introduction of the internal _state field, and the design decision to reset state to the provided init function value rather than undefined. The description further outlines three alternative approaches that were considered and explains the rationale for the chosen approach, demonstrating clear connection to the implementation changes shown in the summary. While not exhaustively detailed, it provides sufficient context to convey meaningful information about the changeset's purpose and design considerations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/nuxt/src/app/composables/state.ts (1)

65-76: Safer key derivation and leaner payload clearing

  • _allKeys should only include $s-prefixed entries; otherwise non‑useState keys (if any) would be transformed and re‑prefixed, causing unintended clears.
  • When removing without reset, prefer deleting the key to avoid retaining enumerable undefined slots (keeps payload slim and prevents stale keys from reappearing in subsequent clears).

Apply:

-  const _allKeys = Object.keys(nuxtApp.payload.state)
-    .map(key => key.substring(useStateKeyPrefix.length))
+  const _allKeys = Object.keys(nuxtApp.payload.state)
+    .filter(key => key.startsWith(useStateKeyPrefix))
+    .map(key => key.substring(useStateKeyPrefix.length))

   for (const _key of _keys) {
-    clearNuxtStateByKey(nuxtApp, useStateKeyPrefix + _key, reset ?? true)
+    clearNuxtStateByKey(nuxtApp, useStateKeyPrefix + _key, reset ?? true)
   }

And streamline per‑key clearing:

-function clearNuxtStateByKey (nuxtApp: NuxtApp, key: string, reset: boolean): void {
-  if (key in nuxtApp.payload.state) {
-    nuxtApp.payload.state[key] = undefined
-  }
-
-  if (nuxtApp._state[key]) {
-    nuxtApp._state[key]!.data.value = reset ? unref(nuxtApp._state[key]!._default()) : undefined
-  }
-}
+function clearNuxtStateByKey (nuxtApp: NuxtApp, key: string, reset: boolean): void {
+  if (reset) {
+    if (nuxtApp._state[key]) {
+      // Resets both _state data and payload via toRef linkage
+      nuxtApp._state[key]!.data.value = unref(nuxtApp._state[key]!._default())
+      return
+    }
+    // No _state registered: remove from payload if present
+    delete nuxtApp.payload.state[key]
+  } else {
+    // Remove from payload and null out internal value if tracked
+    delete nuxtApp.payload.state[key]
+    if (nuxtApp._state[key]) {
+      nuxtApp._state[key]!.data.value = undefined
+    }
+  }
+}
🧹 Nitpick comments (2)
packages/nuxt/src/app/nuxt.ts (1)

145-151: Internal _state map looks good; consider GC of stale keys

The addition is well‑typed and initialised correctly. To avoid long‑lived growth on the client, consider pruning entries (e.g. delete nuxtApp._state[key]) when a state is cleared without reset and no watchers remain. This keeps memory proportional to active state keys.

Also applies to: 319-319

packages/nuxt/src/app/composables/state.ts (1)

45-56: You can simplify to a plain ref; computed indirection is unnecessary

Given _state[key] and state are established above, returning state as Ref<T> is sufficient. The computed fallback to nuxtApp.payload.state[key] is dead code in this path and adds overhead.

Apply:

-  return computed({
-    get () {
-      return nuxtApp._state[key]?.data.value ?? nuxtApp.payload.state[key]
-    },
-    set (value) {
-      if (nuxtApp._state[key]) {
-        nuxtApp._state[key]!.data.value = value
-      } else {
-        nuxtApp.payload.state[key] = value
-      }
-    },
-  })
+  return state as Ref<T>

If you keep the computed, consider at least collapsing the branches to state.value for both get/set. As per coding guidelines.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 568cd34 and e7b2a3b.

📒 Files selected for processing (3)
  • packages/nuxt/src/app/composables/state.ts (2 hunks)
  • packages/nuxt/src/app/nuxt.ts (2 hunks)
  • test/nuxt/composables.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Follow standard TypeScript conventions and best practices

Files:

  • packages/nuxt/src/app/nuxt.ts
  • test/nuxt/composables.test.ts
  • packages/nuxt/src/app/composables/state.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Write unit tests for core functionality using vitest

Files:

  • test/nuxt/composables.test.ts
🧬 Code graph analysis (2)
test/nuxt/composables.test.ts (2)
packages/nuxt/src/app/composables/state.ts (2)
  • clearNuxtState (60-77)
  • useState (20-57)
packages/nuxt/src/app/nuxt.ts (1)
  • useNuxtApp (557-570)
packages/nuxt/src/app/composables/state.ts (1)
packages/nuxt/src/app/nuxt.ts (2)
  • useNuxtApp (557-570)
  • NuxtApp (206-206)
🔇 Additional comments (1)
test/nuxt/composables.test.ts (1)

203-207: Test coverage aligns with intended semantics

The scenarios comprehensively cover payload registration, unwrapped defaults, shared-key behaviour, and reset/remove paths. These rely on useState unref’ing the init return on initialisation; see my comment in state.ts proposing that fix.

Also applies to: 212-216, 217-224, 225-234, 243-285, 286-344

Comment on lines +41 to +44
if (state.value === undefined) {
nuxtApp.payload.state[key] = defaultFn()
}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Initial value should unref the init return (prevents nested refs and matches reset path)

Currently defaultFn() is assigned directly, so passing () => ref(x) yields a ref-of-ref. Reset uses unref(...), so behaviours diverge. Unwrap on first assignment too.

Apply:

-  if (state.value === undefined) {
-    nuxtApp.payload.state[key] = defaultFn()
-  }
+  if (state.value === undefined) {
+    // Unwrap to keep state a plain ref, even if init returns a ref/computed.
+    nuxtApp.payload.state[key] = unref(defaultFn())
+  }
🤖 Prompt for AI Agents
In packages/nuxt/src/app/composables/state.ts around lines 41 to 44, the initial
assignment uses defaultFn() directly which can create a ref-of-ref when
defaultFn returns a ref; change the assignment to unwrap the value (e.g., use
unref(defaultFn()) or equivalent) so the first-assignment mirrors the reset path
and prevents nested refs; ensure unref is imported from Vue if not already.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 20, 2025

CodSpeed Performance Report

Merging #33527 will not alter performance

Comparing Harm-Nullix:reset-to-init-useState (e7b2a3b) with main (b7ed1d3)1

Summary

✅ 10 untouched

Footnotes

  1. No successful run was found on main (568cd34) during the generation of this report, so b7ed1d3 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 20, 2025

Open in StackBlitz

@nuxt/kit

npm i https://pkg.pr.new/@nuxt/kit@33527

nuxt

npm i https://pkg.pr.new/nuxt@33527

@nuxt/rspack-builder

npm i https://pkg.pr.new/@nuxt/rspack-builder@33527

@nuxt/schema

npm i https://pkg.pr.new/@nuxt/schema@33527

@nuxt/vite-builder

npm i https://pkg.pr.new/@nuxt/vite-builder@33527

@nuxt/webpack-builder

npm i https://pkg.pr.new/@nuxt/webpack-builder@33527

commit: e7b2a3b

@Harm-Nullix
Copy link
Author

Quick example addition:

<script setup lang="ts">
import { useState } from '#app'

const stateWithoutKeyAndInit = useState<number | undefined>()
const stateWithKeyWithoutInit = useState<number | undefined>('WithKeyWithoutInit')
const stateWithoutKeyWithInit = useState<any>(() => ({
  spamm: 'eggs',
  green: 'ham',
  value: 1,
}))
const stateWithKeyWithInit = useState<any>('WithKeyWithInit', () => ({
  spamm: 'eggs',
  green: 'ham',
  value: 1,
}))

const resetState = (keys?: string | string[]) => {
  clearNuxtState(keys)
}
const reload = () => window.location.reload()

const state1 = useState('key', () => ref({
  test: 1,
}))
const state2 = useState('key', () => ref({
  test: 2,
}))
console.debug('state1.value :: ', state1.value.test)
console.debug('state2.value :: ', state2.value.test)
state1.value.test = 3
</script>

<template>
  <!-- Edit this file to play around with Nuxt but never commit changes! -->
  <div>
    Nuxt Playground

    <button @click="resetState()">
      Reset state
    </button>
    <button @click="reloadNuxtApp()">
      reload nuxt app
    </button>
    <button @click="reload()">
      reload window
    </button>
  </div>
  <div>
    <ul>
      <li>
        stateWithoutKeyAndInit :: {{ stateWithoutKeyAndInit }}
        <button @click="stateWithoutKeyAndInit = stateWithoutKeyAndInit ? stateWithoutKeyAndInit + 1 :1 ">
          set state
        </button>
      </li>
      <li>
        stateWithKeyWithoutInit :: {{ stateWithKeyWithoutInit }}
        <button @click="stateWithKeyWithoutInit = stateWithKeyWithoutInit ? stateWithKeyWithoutInit + 1 :1 ">
          set state
        </button>
      </li>
      <li>
        stateWithoutKeyWithInit :: {{ stateWithoutKeyWithInit }} <button @click="stateWithoutKeyWithInit.value += 1">
          set state
        </button>
      </li>
      <li>
        stateWithKeyWithInit :: {{ stateWithKeyWithInit }} <button @click="stateWithKeyWithInit.value += 1">
          set state
        </button>
      </li>
    </ul>

    {{ state1 }}. -- <br><br><br>
    {{ state2 }}
  </div>
</template>

<style scoped>

</style>

Copy link
Member

@huang-julien huang-julien left a comment

Choose a reason for hiding this comment

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

I think that it can be really confusing for users and quite hard to debug if there's multiple useState that retuns differents default values

@Harm-Nullix
Copy link
Author

I think that it can be really confusing for users and quite hard to debug if there's multiple useState that retuns differents default values

I agree,
That is how states currently work right?
If you use a key twice, the second useState will just return the current payload of the first useState.

E.g. asyncData has the same logic: When you use asyncData with a default value, and the same key is used based on the url, you will get a other default value (or the current transformed value) from the other component that makes the same call, and not your own default.

You can tackle this by creating a composable around your useState as recommended that would be in charge of this specific key.

I think it is a valid point about data storage and the keys that are being used, I've come across this myself while useing UseFetch with autokeys, but I also think that topic is too brought to pick up in such a change as this, wouldn't you agree?

@Harm-Nullix
Copy link
Author

So what is next?
I am a bit lost in what to do with this now.
I feel like I made a clear case to at least discus.

  • I am not certain of how the PR flow goes (beyond the defaults)

@huang-julien
Copy link
Member

Yes that's just how it currently works with useAsyncData.
I think we can move on with this PR. I'll trigger a discussion with the team but this is probably the best solution.

const useStateKeyPrefix = '$s'
const getDefault = () => undefined

type InitOption<T> = (() => T | Ref<T>)
Copy link
Member

Choose a reason for hiding this comment

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

You can use MaybeRefOrGetter instead


return computed({
get () {
return nuxtApp._state[key]?.data.value ?? nuxtApp.payload.state[key]
Copy link
Member

Choose a reason for hiding this comment

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

We probably don't want to have two possible reference for a state 🤔

if (key in nuxtApp.payload.state) {
nuxtApp.payload.state[key] = undefined
}
clearNuxtStateByKey(nuxtApp, useStateKeyPrefix + _key, reset ?? true)
Copy link
Member

@huang-julien huang-julien Oct 29, 2025

Choose a reason for hiding this comment

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

Let's avoid breaking changes. We can make it true by default for v5 though by using a future flag

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.

2 participants