Skip to content

Added reflection support #227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 9, 2018
Merged

Added reflection support #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a13fc5a
Added reflection support
Feb 19, 2018
cdd6366
fixed review issues
Feb 19, 2018
07eff38
fixed review issues
Feb 19, 2018
49e9c90
fixed corner case with constructor
Feb 20, 2018
614b008
fixed codestyle
Feb 20, 2018
a362138
fixed codestyle in test.ts and reflect.ts to match project codestyle
Mar 2, 2018
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
6 changes: 6 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"chai": "^4.1.2",
"css-loader": "^0.28.9",
"mocha": "^5.0.1",
"reflect-metadata": "^0.1.12",
"rimraf": "^2.6.2",
"rollup": "^0.55.5",
"rollup-plugin-replace": "^2.0.0",
Expand All @@ -60,5 +61,6 @@
"vue-loader": "^14.1.1",
"vue-template-compiler": "^2.5.13",
"webpack": "^3.11.0"
}
},
"dependencies": {}
}
32 changes: 30 additions & 2 deletions src/component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Vue, { ComponentOptions } from 'vue'
import { copyReflectionMetadata, reflectionIsSupported, ReflectionMap } from './reflect'
import { VueClass, DecoratedClass } from './declarations'
import { collectDataFromConstructor } from './data'
import { hasProto, isPrimitive, warn } from './util'
Expand All @@ -23,13 +24,27 @@ export function componentFactory (
Component: VueClass<Vue>,
options: ComponentOptions<Vue> = {}
): VueClass<Vue> {
const reflectionMap: ReflectionMap = {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need such temporary object and to process for it in different functions? Why don't we just directly assign the original metadata to component properties?

Copy link
Author

@realglebivanov realglebivanov Feb 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Do you mean ComponentOptions? If you mean that, it's a tricky way because i don't want to change options interface and expose internal work of Component decorator.

Copy link
Member

Choose a reason for hiding this comment

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

No, I don't mean that.
I mean why we need to store metadata in temporary object rather than just forward them directly on the place.
https://github.com/vuejs/vue-class-component/pull/227/files#diff-7db1d7c8dc88ce00d1aa236fa0c128ccR40
https://github.com/vuejs/vue-class-component/pull/227/files#diff-7db1d7c8dc88ce00d1aa236fa0c128ccR118

Copy link
Author

@realglebivanov realglebivanov Feb 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Because forwarding code will be duplicated. If you okay with that, I'll do it in the way you are asking. Also, we don't have a new class object until almost the end of function.

Copy link
Member

Choose a reason for hiding this comment

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

Well, then we can extract the logic as a function. I believe we can remove reflectionMap without duplication.

something like:

function forwardMetadata(to: typeof Vue, from: typeof Vue, propertyKey: string): void {
  Reflect.getOwnMetadataKeys(from, propertyKey).forEach(metaKey => {
    const metadata = Reflect.getOwnMetadata(metaKey, from, propertyKey)
    Reflect.defineMetadata(metaKey, metadata, to, propertyKey)
  })
}

Then we just call it inline:

  Object.getOwnPropertyNames(Original).forEach(key => {
     // `prototype` should not be overwritten
     if (key === 'prototype') {
       return
     }
 	 
-    reflectionMap.static[key] = Reflect.getOwnMetadataKeys(Original, key);
+    forwardMetadata(Extended, Original, key)

     // Some browsers does not allow reconfigure built-in properties
     const extendedDescriptor = Object.getOwnPropertyDescriptor(Extended, key)
     if (extendedDescriptor && !extendedDescriptor.configurable) {
  forwardStaticMembersAndCollectReflection(Extended, Component, Super, reflectionMap)
-  copyReflectionMetadata(Component, Extended, reflectionMap)
+  Object.getOwnPropertyNames(proto).forEach(key => {
+    forwardMetadata(Extended.prototype, proto, key)
+  })

instance: {},
static: {},
constructor: []
}

if (reflectionIsSupported()) {
reflectionMap.constructor = Reflect.getOwnMetadataKeys(Component)
}
options.name = options.name || (Component as any)._componentTag || (Component as any).name
// prototype props.
const proto = Component.prototype
Object.getOwnPropertyNames(proto).forEach(function (key) {
if (key === 'constructor') {
return
}

if (reflectionIsSupported()) {
reflectionMap.instance[key] = Reflect.getOwnMetadataKeys(proto, key)
}

// hooks
if ($internalHooks.indexOf(key) > -1) {
options[key] = proto[key]
Expand Down Expand Up @@ -69,7 +84,11 @@ export function componentFactory (
: Vue
const Extended = Super.extend(options)

forwardStaticMembers(Extended, Component, Super)
forwardStaticMembersAndCollectReflection(Extended, Component, Super, reflectionMap)

if (reflectionIsSupported()) {
copyReflectionMetadata(Component, Extended, reflectionMap)
}

return Extended
}
Expand All @@ -93,14 +112,23 @@ const reservedPropertyNames = [
'filter'
]

function forwardStaticMembers (Extended: typeof Vue, Original: typeof Vue, Super: typeof Vue): void {
function forwardStaticMembersAndCollectReflection (
Extended: typeof Vue,
Original: typeof Vue,
Super: typeof Vue,
reflectionMap: ReflectionMap
): void {
// We have to use getOwnPropertyNames since Babel registers methods as non-enumerable
Object.getOwnPropertyNames(Original).forEach(key => {
// `prototype` should not be overwritten
if (key === 'prototype') {
return
}

if (reflectionIsSupported()) {
reflectionMap.static[key] = Reflect.getOwnMetadataKeys(Original, key)
}

// Some browsers does not allow reconfigure built-in properties
const extendedDescriptor = Object.getOwnPropertyDescriptor(Extended, key)
if (extendedDescriptor && !extendedDescriptor.configurable) {
Expand Down
27 changes: 26 additions & 1 deletion src/globals.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,33 @@
* should not expose to userland
*/

declare namespace Reflect {
function decorate(decorators: ClassDecorator[], target: Function): Function
function decorate(decorators: (PropertyDecorator | MethodDecorator)[], target: Object, propertyKey: string | symbol, attributes?: PropertyDescriptor): PropertyDescriptor
function metadata(metadataKey: any, metadataValue: any): {
(target: Function): void
(target: Object, propertyKey: string | symbol): void
}
function defineMetadata(metadataKey: any, metadataValue: any, target: Object): void
function defineMetadata(metadataKey: any, metadataValue: any, target: Object, propertyKey: string | symbol): void
function hasMetadata(metadataKey: any, target: Object): boolean
function hasMetadata(metadataKey: any, target: Object, propertyKey: string | symbol): boolean
function hasOwnMetadata(metadataKey: any, target: Object): boolean
function hasOwnMetadata(metadataKey: any, target: Object, propertyKey: string | symbol): boolean
function getMetadata(metadataKey: any, target: Object): any
function getMetadata(metadataKey: any, target: Object, propertyKey: string | symbol): any
function getOwnMetadata(metadataKey: any, target: Object): any
function getOwnMetadata(metadataKey: any, target: Object, propertyKey: string | symbol): any
function getMetadataKeys(target: Object): any[]
function getMetadataKeys(target: Object, propertyKey: string | symbol): any[]
function getOwnMetadataKeys(target: Object): any[]
function getOwnMetadataKeys(target: Object, propertyKey: string | symbol): any[]
function deleteMetadata(metadataKey: any, target: Object): boolean
function deleteMetadata(metadataKey: any, target: Object, propertyKey: string | symbol): boolean
}

declare const process: {
env: {
NODE_ENV: string
}
}
}
39 changes: 39 additions & 0 deletions src/reflect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { VueConstructor } from 'vue'

export type StringToArrayMap = {
[key: string]: Array<string>
}

export type ReflectionMap = {
constructor: Array<string>,
instance: StringToArrayMap,
static: StringToArrayMap
}

export function reflectionIsSupported () {
return (Reflect && Reflect.defineMetadata) !== undefined
}

export function copyReflectionMetadata (
from: VueConstructor,
to: VueConstructor,
reflectionMap: ReflectionMap
) {
shallowCopy(from.prototype, to.prototype, reflectionMap.instance)
shallowCopy(from, to, reflectionMap.static)
shallowCopy(from, to, {'constructor': reflectionMap.constructor})
}

function shallowCopy (from: VueConstructor, to: VueConstructor, propertyKeys: StringToArrayMap) {
for (const propertyKey in propertyKeys) {
propertyKeys[propertyKey].forEach((metadataKey) => {
if (propertyKey == 'constructor') {
const metadata = Reflect.getOwnMetadata(metadataKey, from)
Reflect.defineMetadata(metadataKey, metadata, to)
} else {
const metadata = Reflect.getOwnMetadata(metadataKey, from, propertyKey)
Reflect.defineMetadata(metadataKey, metadata, to, propertyKey)
}
})
}
}
31 changes: 31 additions & 0 deletions test/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'reflect-metadata';
import Component, { createDecorator, mixins } from '../lib'
import { expect } from 'chai'
import * as td from 'testdouble'
Expand Down Expand Up @@ -369,4 +370,34 @@ describe('vue-class-component', () => {
expect(vm.valueA).to.equal('hi')
expect(vm.valueB).to.equal(456)
})

it('copies reflection metadata', function () {
@Component
@Reflect.metadata('worksConstructor', true)
class Test extends Vue {
@Reflect.metadata('worksStatic', true)
static staticValue: string = 'staticValue'

private _test: boolean = false;

@Reflect.metadata('worksMethod', true)
test (): void {
void 0
}

@Reflect.metadata('worksAccessor', true)
get testAccessor (): boolean {
return this._test
}

set testAccessor (value: boolean) {
this._test = value
}
}

expect(Reflect.getOwnMetadata('worksConstructor', Test)).to.equal(true)
expect(Reflect.getOwnMetadata('worksStatic', Test, 'staticValue')).to.equal(true)
expect(Reflect.getOwnMetadata('worksMethod', Test.prototype, 'test')).to.equal(true)
expect(Reflect.getOwnMetadata('worksAccessor', Test.prototype, 'testAccessor')).to.equal(true)
})
})