Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions packages/runtime-core/__tests__/apiInject.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
hasInjectionContext,
inject,
nextTick,
onBeforeUpdate,
onMounted,
provide,
reactive,
readonly,
Expand Down Expand Up @@ -347,6 +349,104 @@ describe('api: provide/inject', () => {
expect(serialize(root)).toBe(`<div><!----></div>`)
})

// #13921
it('overlapping inheritance cycles', async () => {
let shouldProvide = ref(false)

const Comp4 = {
props: ['data'],
setup() {
const data = ref('foo -1')

onMounted(() => {
data.value = inject('foo', 'foo 0')
})

onBeforeUpdate(() => {
data.value = inject('foo', 'foo 0')
})

return () => [h('div', data.value)]
},
}

const Comp3 = {
props: ['data'],
setup() {
const data = ref('foo -1')

onMounted(() => {
data.value = inject('foo', 'foo 0')
})

onBeforeUpdate(() => {
data.value = inject('foo', 'foo 0')
})

return () => [
h('div', data.value),
h(Comp4, { data: shouldProvide.value }),
]
},
}

const Comp2 = {
setup() {
const data = ref('foo -1')

onMounted(() => {
data.value = inject('foo', 'foo 0')
})

onBeforeUpdate(() => {
if (shouldProvide.value) {
provide('foo', 'foo 2')
}

data.value = inject('foo', 'foo 0')
})

return () => [
h('div', data.value),
h(Comp3, { data: shouldProvide.value }),
]
},
}

const Comp1 = {
setup() {
provide('foo', 'foo 1')
const data = ref('foo -1')

onMounted(() => {
data.value = inject('foo', 'foo 0')
})

onBeforeUpdate(() => {
data.value = inject('foo', 'foo 0')
})

return () => [h('div', data.value), h(Comp2)]
},
}

const root = nodeOps.createElement('div')
render(h(Comp1), root)

shouldProvide.value = true
await nextTick()

/*
First (Root Component) should be "foo 0" because it is the Root Component and provdes shall only be injected to Descandents.
Second (Component 2) should be "foo 1" because it should inherit the provide from the Root Component
Third (Component 3) should be "foo 2" because it should inherit the provide from Component 2 (in the second render when shouldProvide = true)
Fourth (Component 4) should also be "foo 2" because it should inherit the provide from Component 3 which should inherit it from Component 2 (in the second render when shouldProvide = true)
*/
expect(serialize(root)).toBe(
`<div><div>foo 0</div><div>foo 1</div><div>foo 2</div><div>foo 2</div></div>`,
)
})

describe('hasInjectionContext', () => {
it('should be false outside of setup', () => {
expect(hasInjectionContext()).toBe(false)
Expand Down
13 changes: 3 additions & 10 deletions packages/runtime-core/src/apiInject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,12 @@ export function provide<T, K = InjectionKey<T> | string | number>(
warn(`provide() can only be used inside setup().`)
}
} else {
let provides = currentInstance.provides
// by default an instance inherits its parent's provides object
// but when it needs to provide values of its own, it creates its
// own provides object using parent provides object as prototype.
// by default an instance it creates its own provides object
// using parent provides object as prototype.
// this way in `inject` we can simply look up injections from direct
// parent and let the prototype chain do the work.
const parentProvides =
currentInstance.parent && currentInstance.parent.provides
if (parentProvides === provides) {
provides = currentInstance.provides = Object.create(parentProvides)
}
// TS doesn't allow symbol as index type
provides[key as string] = value
currentInstance.provides[key as string] = value
Comment on lines +19 to +24
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

Restore the per-instance provides fork before writing.

currentInstance.provides still aliases parent.provides until we fork it (see createComponentInstance in component.ts). With this change we now write straight into the parent’s map, so a child re-provide will bleed into siblings and ancestors. We need to restore the Object.create(parentProvides) guard (or otherwise ensure each instance owns its own map) before assigning the new value.

-    // by default an instance it creates its own provides object
-    // using parent provides object as prototype.
-    // this way in `inject` we can simply look up injections from direct
-    // parent and let the prototype chain do the work.
-    // TS doesn't allow symbol as index type
-    currentInstance.provides[key as string] = value
+    let { provides, parent } = currentInstance
+    if (parent) {
+      const parentProvides = parent.provides
+      if (provides === parentProvides) {
+        provides = currentInstance.provides = Object.create(parentProvides)
+      }
+    }
+    // TS doesn't allow symbol as index type
+    provides[key as string] = value
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// by default an instance it creates its own provides object
// using parent provides object as prototype.
// this way in `inject` we can simply look up injections from direct
// parent and let the prototype chain do the work.
const parentProvides =
currentInstance.parent && currentInstance.parent.provides
if (parentProvides === provides) {
provides = currentInstance.provides = Object.create(parentProvides)
}
// TS doesn't allow symbol as index type
provides[key as string] = value
currentInstance.provides[key as string] = value
let { provides, parent } = currentInstance
if (parent) {
const parentProvides = parent.provides
if (provides === parentProvides) {
provides = currentInstance.provides = Object.create(parentProvides)
}
}
// TS doesn't allow symbol as index type
provides[key as string] = value
🤖 Prompt for AI Agents
In packages/runtime-core/src/apiInject.ts around lines 19 to 24, the code writes
directly into currentInstance.provides which still aliases parent.provides;
restore the per-instance provides fork before assigning so we don't mutate the
parent's map. Detect when currentInstance.provides === parent.provides (or when
it shares reference) and replace it with a shallow clone created from the parent
(e.g., Object.create(parentProvides)) so the instance owns its own map, then
assign the key (converting symbol keys to string as needed) into that
per-instance provides object.

}
}

Expand Down
5 changes: 4 additions & 1 deletion packages/runtime-core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,10 @@ export function createComponentInstance(
exposeProxy: null,
withProxy: null,

provides: parent ? parent.provides : Object.create(appContext.provides),
// component instance always creates a new Provides object with prototype of parent provides (or app/global provides when there is no parent instance) so to ensure that Parent provides will always be inherited, even when parent provides after the fact.
provides: parent
? Object.create(parent.provides)
Copy link
Member

Choose a reason for hiding this comment

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

The above comment should be added here

Copy link
Author

Choose a reason for hiding this comment

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

The original comment was in apiInject.ts, but I can copy or move it to there too.

: Object.create(appContext.provides),
ids: parent ? parent.ids : ['', 0, 0],
accessCache: null!,
renderCache: [],
Expand Down