Skip to content

Commit

Permalink
fix(runtime-core): fix resolving assets from mixins and extends
Browse files Browse the repository at this point in the history
fix #1963
  • Loading branch information
yyx990803 committed Aug 26, 2020
1 parent bc64c60 commit 0cb7f7f
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 3 deletions.
58 changes: 58 additions & 0 deletions packages/runtime-core/__tests__/helpers/resolveAssets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,62 @@ describe('resolveAssets', () => {
expect(serializeInner(root)).toBe('<div>hello</div>')
})
})

test('resolving from mixins & extends', () => {
const FooBar = () => null
const BarBaz = { mounted: () => null }

let component1: Component | string
let component2: Component | string
let component3: Component | string
let component4: Component | string
let directive1: Directive
let directive2: Directive
let directive3: Directive
let directive4: Directive

const Base = {
components: {
FooBar: FooBar
}
}
const Mixin = {
directives: {
BarBaz: BarBaz
}
}

const Root = {
extends: Base,
mixins: [Mixin],
setup() {
return () => {
component1 = resolveComponent('FooBar')!
directive1 = resolveDirective('BarBaz')!
// camelize
component2 = resolveComponent('Foo-bar')!
directive2 = resolveDirective('Bar-baz')!
// capitalize
component3 = resolveComponent('fooBar')!
directive3 = resolveDirective('barBaz')!
// camelize and capitalize
component4 = resolveComponent('foo-bar')!
directive4 = resolveDirective('bar-baz')!
}
}
}

const app = createApp(Root)
const root = nodeOps.createElement('div')
app.mount(root)
expect(component1!).toBe(FooBar)
expect(component2!).toBe(FooBar)
expect(component3!).toBe(FooBar)
expect(component4!).toBe(FooBar)

expect(directive1!).toBe(BarBaz)
expect(directive2!).toBe(BarBaz)
expect(directive3!).toBe(BarBaz)
expect(directive4!).toBe(BarBaz)
})
})
20 changes: 18 additions & 2 deletions packages/runtime-core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Slots, initSlots, InternalSlots } from './componentSlots'
import { warn } from './warning'
import { ErrorCodes, callWithErrorHandling } from './errorHandling'
import { AppContext, createAppContext, AppConfig } from './apiCreateApp'
import { validateDirectiveName } from './directives'
import { Directive, validateDirectiveName } from './directives'
import { applyOptions, ComponentOptions } from './componentOptions'
import {
EmitsOptions,
Expand Down Expand Up @@ -221,6 +221,17 @@ export interface ComponentInternalInstance {
*/
renderCache: (Function | VNode)[]

/**
* Resolved component registry, only for components with mixins or extends
* @internal
*/
components: Record<string, ConcreteComponent> | null
/**
* Resolved directive registry, only for components with mixins or extends
* @internal
*/
directives: Record<string, Directive> | null

// the rest are only for stateful components ---------------------------------

/**
Expand Down Expand Up @@ -372,6 +383,10 @@ export function createComponentInstance(
accessCache: null!,
renderCache: [],

// local resovled assets
components: null,
directives: null,

// state
ctx: EMPTY_OBJ,
data: EMPTY_OBJ,
Expand Down Expand Up @@ -733,7 +748,8 @@ export function formatComponentName(
}
name =
inferFromRegistry(
(instance.parent.type as ComponentOptions).components
instance.components ||
(instance.parent.type as ComponentOptions).components
) || inferFromRegistry(instance.appContext.components)
}

Expand Down
29 changes: 29 additions & 0 deletions packages/runtime-core/src/componentOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,9 @@ export function applyOptions(
watch: watchOptions,
provide: provideOptions,
inject: injectOptions,
// assets
components,
directives,
// lifecycle
beforeMount,
mounted,
Expand Down Expand Up @@ -568,6 +571,32 @@ export function applyOptions(
}
}

// asset options.
// To reduce memory usage, only components with mixins or extends will have
// resolved asset registry attached to instance.
if (asMixin) {

This comment has been minimized.

Copy link
@cool-little-fish

cool-little-fish Aug 27, 2020

Contributor

It seems this will make the component's extends or mixin make priority than itself @yyx990803

if (components) {
extend(
instance.components ||
(instance.components = extend(
{},
(instance.type as ComponentOptions).components
) as Record<string, ConcreteComponent>),
components
)
}
if (directives) {
extend(
instance.directives ||
(instance.directives = extend(
{},
(instance.type as ComponentOptions).directives
)),
directives
)
}
}

// lifecycle options
if (!asMixin) {
callSyncHook('created', options, publicThis, globalMixins)
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime-core/src/helpers/resolveAssets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ function resolveAsset(

const res =
// local registration
resolve((Component as ComponentOptions)[type], name) ||
// check instance[type] first for components with mixin or extends.
resolve(instance[type] || (Component as ComponentOptions)[type], name) ||
// global registration
resolve(instance.appContext[type], name)
if (__DEV__ && warnMissing && !res) {
Expand Down

2 comments on commit 0cb7f7f

@cool-little-fish
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @yyx990803 , may I ask a question about this pr.
Why the line btw 184-194 in resolveAssets.spec.ts file, there is a ! symbol as suffix.
Thanks for help~

@cool-little-fish
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.