Skip to content

Resolve annotation sets in setup so the Import menu cannot throw#1762

Merged
BryonLewis merged 2 commits into
mainfrom
fix/annotation-sets-inject-outside-setup
Jul 16, 2026
Merged

Resolve annotation sets in setup so the Import menu cannot throw#1762
BryonLewis merged 2 commits into
mainfrom
fix/annotation-sets-inject-outside-setup

Conversation

@mattdawkins

Copy link
Copy Markdown
Member

Problem

ImportAnnotations calls useAnnotationSets() — and therefore inject() — from inside the sets computed:

const sets = computed(() => {
  const data = useAnnotationSets();   // inject() in a computed getter
  const temp = cloneDeep(data.value);
  temp.push('default');
  return temp;
});

inject() resolves against the current component instance. Outside setup(), Vue 2.7 only sets that during render (core/instance/render.ts calls setCurrentInstance(vm) around _render). So the getter works when the template evaluates it, and throws for every other caller:

Missing provided object for symbol Symbol(annotationSets): must provideAnnotator()

The Import menu's contents are lazy — v-menu doesn't render them until opened — so the first read of sets already happens off the render path. Any future watcher, effect, or programmatic read hits the same throw. It survives today only by accident of when Vue happens to evaluate the computed, which is not a property this code controls.

This was found while debugging a separate issue: reading sets from a debugger (i.e. outside a render pass) threw the error above on unmodified main.

Fix

Hoist the lookup into setup() and close over the resulting ref. Behaviour is unchanged: clone the provided sets, append default, leave the provided array untouched.

Test

Adds importAnnotationsSets.spec.ts, which mounts the component with the menu closed — reproducing the off-render read — and asserts sets resolves.

Verified the test is meaningful rather than vacuous: against the previous code all three cases fail with

AssertionError: expected [Function] to not throw an error but
'Error: Missing provided object for symbol Symbol(annotationSets)…' was thrown

Verification

  • npm test — 609 passing (main's suite plus the 3 new cases)
  • npx eslint on both changed files — clean
  • npm run build:web — clean

Note for reviewers

npm test requires Node ≥20 since #1758 bumped vitest to 4 (engines: ^20 || ^22 || >=24); it fails to start on Node 18 with ERR_REQUIRE_ESM. client/README.md still says "Requires Node 18+" — stale, but out of scope here.

There is a second, separate defect in this file that I deliberately did not touch: const currentSet = ref(defaultSet || 'default') (line ~143) is missing .value. defaultSet is a ref, so it's always truthy and ref() passes an existing ref straight through — currentSet therefore aliases the injected annotationSet ref rather than copying its value, and the || 'default' fallback never applies. Fixing it would change behaviour (the Annotation Set combobox is gated on currentSet !== '', so it would start rendering where it currently doesn't), so it wants its own PR and a decision about intent.

🤖 Generated with Claude Code

ImportAnnotations called useAnnotationSets() -- and so inject() -- from
inside the `sets` computed. inject() resolves against the current
component instance, which outside setup() is only set during render, so
the getter worked when the template evaluated it and threw

  Missing provided object for symbol Symbol(annotationSets):
  must provideAnnotator()

for every other caller. The Import menu's contents are lazy, so the
first read of `sets` happens off the render path, and any future
watcher, effect or programmatic read of `sets` would hit the same throw.
It survives today only by accident of when Vue happens to evaluate it.

Hoist the lookup into setup() and close over the resulting ref. The
computed keeps its behaviour: clone the provided sets, append 'default',
and leave the provided array untouched.

Add a regression test that mounts the component with the menu closed --
reproducing the off-render read -- and asserts `sets` resolves. It fails
on the previous code with the error above.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mattdawkins
mattdawkins requested a review from BryonLewis July 16, 2026 15:12

@BryonLewis BryonLewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fixed the other issue referenced in the PR description and updated some documentation about node versions.

@BryonLewis
BryonLewis merged commit e268b0c into main Jul 16, 2026
3 checks passed
@BryonLewis
BryonLewis deleted the fix/annotation-sets-inject-outside-setup branch July 16, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants