Skip to content

Commit

Permalink
fix(build): fix component resolution when disabling options API
Browse files Browse the repository at this point in the history
fix #1688
  • Loading branch information
yyx990803 committed Jul 23, 2020
1 parent ba17c87 commit a75b8a2
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 47 deletions.
37 changes: 14 additions & 23 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 { Directive, validateDirectiveName } from './directives'
import { validateDirectiveName } from './directives'
import { applyOptions, ComponentOptions } from './componentOptions'
import {
EmitsOptions,
Expand Down Expand Up @@ -223,17 +223,6 @@ export interface ComponentInternalInstance {
*/
renderCache: (Function | VNode)[]

/**
* Asset hashes that prototypally inherits app-level asset hashes for fast
* resolution
* @internal
*/
components: Record<string, Component>
/**
* @internal
*/
directives: Record<string, Directive>

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

// main proxy that serves as the public instance (`this`)
Expand Down Expand Up @@ -354,15 +343,17 @@ export function createComponentInstance(
parent: ComponentInternalInstance | null,
suspense: SuspenseBoundary | null
) {
const type = vnode.type as Component
// inherit parent app context - or - if root, adopt from root vnode
const appContext =
(parent ? parent.appContext : vnode.appContext) || emptyAppContext

const instance: ComponentInternalInstance = {
uid: uid++,
vnode,
type,
parent,
appContext,
type: vnode.type as Component,
root: null!, // to be immediately set
next: null,
subTree: null!, // will be set synchronously right after creation
Expand All @@ -385,10 +376,6 @@ export function createComponentInstance(
setupState: EMPTY_OBJ,
setupContext: null,

// per-instance asset storage (mutable during options resolution)
components: Object.create(appContext.components),
directives: Object.create(appContext.directives),

// suspense related
suspense,
asyncDep: null,
Expand Down Expand Up @@ -727,14 +714,18 @@ export function formatComponentName(
}

if (!name && instance && instance.parent) {
// try to infer the name based on local resolution
const registry = instance.parent.components
for (const key in registry) {
if (registry[key] === Component) {
name = key
break
// try to infer the name based on reverse resolution
const inferFromRegistry = (registry: Record<string, any> | undefined) => {
for (const key in registry) {
if (registry[key] === Component) {
return key
}
}
}
name =
inferFromRegistry(
(instance.parent.type as ComponentOptions).components
) || inferFromRegistry(instance.appContext.components)
}

return name ? classify(name) : isRoot ? `App` : `Anonymous`
Expand Down
11 changes: 0 additions & 11 deletions packages/runtime-core/src/componentOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,6 @@ export function applyOptions(
watch: watchOptions,
provide: provideOptions,
inject: injectOptions,
// assets
components,
directives,
// lifecycle
beforeMount,
mounted,
Expand Down Expand Up @@ -570,14 +567,6 @@ export function applyOptions(
}
}

// asset options
if (components) {
extend(instance.components, components)
}
if (directives) {
extend(instance.directives, directives)
}

// lifecycle options
if (!asMixin) {
callSyncHook('created', options, publicThis, globalMixins)
Expand Down
43 changes: 30 additions & 13 deletions packages/runtime-core/src/helpers/resolveAssets.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { currentRenderingInstance } from '../componentRenderUtils'
import { currentInstance, Component, FunctionalComponent } from '../component'
import {
currentInstance,
Component,
FunctionalComponent,
ComponentOptions
} from '../component'
import { Directive } from '../directives'
import { camelize, capitalize, isString } from '@vue/shared'
import { warn } from '../warning'
Expand Down Expand Up @@ -58,24 +63,27 @@ function resolveAsset(
) {
const instance = currentRenderingInstance || currentInstance
if (instance) {
let camelized, capitalized
const registry = instance[type]
let res =
registry[name] ||
registry[(camelized = camelize(name))] ||
registry[(capitalized = capitalize(camelized))]
if (!res && type === COMPONENTS) {
const self = instance.type
const selfName = (self as FunctionalComponent).displayName || self.name
const Component = instance.type

// self name has highest priority
if (type === COMPONENTS) {
const selfName =
(Component as FunctionalComponent).displayName || Component.name
if (
selfName &&
(selfName === name ||
selfName === camelized ||
selfName === capitalized)
selfName === camelize(name) ||
selfName === capitalize(camelize(name)))
) {
res = self
return Component
}
}

const res =
// local registration
resolve((Component as ComponentOptions)[type], name) ||
// global registration
resolve(instance.appContext[type], name)
if (__DEV__ && warnMissing && !res) {
warn(`Failed to resolve ${type.slice(0, -1)}: ${name}`)
}
Expand All @@ -87,3 +95,12 @@ function resolveAsset(
)
}
}

function resolve(registry: Record<string, any> | undefined, name: string) {
return (
registry &&
(registry[name] ||
registry[camelize(name)] ||
registry[capitalize(camelize(name))])
)
}

3 comments on commit a75b8a2

@yyx990803
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for @vue/test-utils: instance.components is now removed because the component resolution now directly checks instance.type.components (local) and then fallbacks to instance.appContext.components (global). This removes the constant cost of creating two extra asset objects on each instance.

This may affect VTU's stubbing behavior but it should be possible to fix it while retaining backwards compat: https://github.com/vuejs/vue-test-utils-next/blob/master/src/stubs.ts#L100

/cc @cexbrayat @lmiller1990 @dobromir-hristov @JessicaSachs @afontcu

@cexbrayat
Copy link
Member

Choose a reason for hiding this comment

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

@yyx990803 Thanks for the heads up! I think we're good for the code you linked IIUC, as I was already checking instance.type.components? But the matches logic in findComponent will need to be fixed https://github.com/vuejs/vue-test-utils-next/blob/master/src/utils/find.ts#L45
We'll check it out when rc.5 is released

@lmiller1990
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the heads up @yyx990803, we will keep on top of this. I think you are right @cexbrayat about findComponent, but it looks like it should be a pretty straight forward fix.

Please sign in to comment.