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 1 commit
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
28 changes: 26 additions & 2 deletions dist/vue-class-component.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,25 @@ Object.defineProperty(exports, '__esModule', { value: true });

function _interopDefault (ex) { return (ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex; }

require('reflect-metadata');
var Vue = _interopDefault(require('vue'));

function copyReflectionMetadata(from, to, reflectionMap) {
shallowCopy(from.prototype, to.prototype, reflectionMap.instance);
shallowCopy(from, to, reflectionMap.static);
}
function shallowCopy(from, to, propertyKeys) {
var _loop_1 = function (propertyKey) {
propertyKeys[propertyKey].forEach(function (metadataKey) {
var metadata = Reflect.getOwnMetadata(metadataKey, from, propertyKey);
Reflect.defineMetadata(metadataKey, metadata, to, propertyKey);
});
};
for (var propertyKey in propertyKeys) {
_loop_1(propertyKey);
}
}

var hasProto = { __proto__: [] } instanceof Array;
function createDecorator(factory) {
return function (target, key, index) {
Expand Down Expand Up @@ -105,13 +122,18 @@ var $internalHooks = [
];
function componentFactory(Component, options) {
if (options === void 0) { options = {}; }
var reflectionMap = {
instance: {},
static: {}
};
options.name = options.name || Component._componentTag || Component.name;
// prototype props.
var proto = Component.prototype;
Object.getOwnPropertyNames(proto).forEach(function (key) {
if (key === 'constructor') {
return;
}
reflectionMap.instance[key] = Reflect.getOwnMetadataKeys(proto, key);
// hooks
if ($internalHooks.indexOf(key) > -1) {
options[key] = proto[key];
Expand Down Expand Up @@ -147,7 +169,8 @@ function componentFactory(Component, options) {
? superProto.constructor
: Vue;
var Extended = Super.extend(options);
forwardStaticMembers(Extended, Component, Super);
forwardStaticMembersAndCollectReflection(Extended, Component, Super, reflectionMap);
copyReflectionMetadata(Component, Extended, reflectionMap);
return Extended;
}
var reservedPropertyNames = [
Expand All @@ -165,13 +188,14 @@ var reservedPropertyNames = [
'directive',
'filter'
];
function forwardStaticMembers(Extended, Original, Super) {
function forwardStaticMembersAndCollectReflection(Extended, Original, Super, reflectionMap) {
// We have to use getOwnPropertyNames since Babel registers methods as non-enumerable
Object.getOwnPropertyNames(Original).forEach(function (key) {
// `prototype` should not be overwritten
if (key === 'prototype') {
return;
}
reflectionMap.static[key] = Reflect.getOwnMetadataKeys(Original, key);
// Some browsers does not allow reconfigure built-in properties
var extendedDescriptor = Object.getOwnPropertyDescriptor(Extended, key);
if (extendedDescriptor && !extendedDescriptor.configurable) {
Expand Down
35 changes: 29 additions & 6 deletions dist/vue-class-component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,29 @@
* @license MIT
*/
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('vue')) :
typeof define === 'function' && define.amd ? define(['exports', 'vue'], factory) :
(factory((global.VueClassComponent = {}),global.Vue));
}(this, (function (exports,Vue) { 'use strict';
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('reflect-metadata'), require('vue')) :
typeof define === 'function' && define.amd ? define(['exports', 'reflect-metadata', 'vue'], factory) :
(factory((global.VueClassComponent = {}),null,global.Vue));
}(this, (function (exports,reflectMetadata,Vue) { 'use strict';

Vue = Vue && Vue.hasOwnProperty('default') ? Vue['default'] : Vue;

function copyReflectionMetadata(from, to, reflectionMap) {
shallowCopy(from.prototype, to.prototype, reflectionMap.instance);
shallowCopy(from, to, reflectionMap.static);
}
function shallowCopy(from, to, propertyKeys) {
var _loop_1 = function (propertyKey) {
propertyKeys[propertyKey].forEach(function (metadataKey) {
var metadata = Reflect.getOwnMetadata(metadataKey, from, propertyKey);
Reflect.defineMetadata(metadataKey, metadata, to, propertyKey);
});
};
for (var propertyKey in propertyKeys) {
_loop_1(propertyKey);
}
}

var hasProto = { __proto__: [] } instanceof Array;
function createDecorator(factory) {
return function (target, key, index) {
Expand Down Expand Up @@ -105,13 +121,18 @@ var $internalHooks = [
];
function componentFactory(Component, options) {
if (options === void 0) { options = {}; }
var reflectionMap = {
instance: {},
static: {}
};
options.name = options.name || Component._componentTag || Component.name;
// prototype props.
var proto = Component.prototype;
Object.getOwnPropertyNames(proto).forEach(function (key) {
if (key === 'constructor') {
return;
}
reflectionMap.instance[key] = Reflect.getOwnMetadataKeys(proto, key);
// hooks
if ($internalHooks.indexOf(key) > -1) {
options[key] = proto[key];
Expand Down Expand Up @@ -147,7 +168,8 @@ function componentFactory(Component, options) {
? superProto.constructor
: Vue;
var Extended = Super.extend(options);
forwardStaticMembers(Extended, Component, Super);
forwardStaticMembersAndCollectReflection(Extended, Component, Super, reflectionMap);
copyReflectionMetadata(Component, Extended, reflectionMap);
return Extended;
}
var reservedPropertyNames = [
Expand All @@ -165,13 +187,14 @@ var reservedPropertyNames = [
'directive',
'filter'
];
function forwardStaticMembers(Extended, Original, Super) {
function forwardStaticMembersAndCollectReflection(Extended, Original, Super, reflectionMap) {
// We have to use getOwnPropertyNames since Babel registers methods as non-enumerable
Object.getOwnPropertyNames(Original).forEach(function (key) {
// `prototype` should not be overwritten
if (key === 'prototype') {
return;
}
reflectionMap.static[key] = Reflect.getOwnMetadataKeys(Original, key);
// Some browsers does not allow reconfigure built-in properties
var extendedDescriptor = Object.getOwnPropertyDescriptor(Extended, key);
if (extendedDescriptor && !extendedDescriptor.configurable) {
Expand Down
2 changes: 1 addition & 1 deletion dist/vue-class-component.min.js
View file
Open in desktop

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

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.

3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,8 @@
"vue-loader": "^14.1.1",
"vue-template-compiler": "^2.5.13",
"webpack": "^3.11.0"
},
"dependencies": {
"reflect-metadata": "^0.1.12"
}
}
20 changes: 18 additions & 2 deletions src/component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import 'reflect-metadata'
Copy link
Member

@ktsn ktsn Feb 19, 2018

Choose a reason for hiding this comment

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

I'm not a fan of adding dependency that some users may not need as it bloats the lib size. Can we remove the dep and make this feature be optional?
I think only forwarding metadata when Reflect metadata is supported would work.

Copy link
Author

@realglebivanov realglebivanov Feb 19, 2018

Choose a reason for hiding this comment

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

That's reasonable, I'll fix it.

import Vue, { ComponentOptions } from 'vue'
import { copyReflectionMetadata, ReflectionMap } from './reflect'
import { VueClass, DecoratedClass } from './declarations'
import { collectDataFromConstructor } from './data'
import { hasProto, isPrimitive, warn } from './util'
Expand All @@ -23,13 +25,19 @@ export function componentFactory (
Component: VueClass<Vue>,
options: ComponentOptions<Vue> = {}
): VueClass<Vue> {
const reflectionMap: ReflectionMap = {
Copy link
Member

@ktsn ktsn Feb 19, 2018

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

@ktsn ktsn Feb 19, 2018

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

@ktsn ktsn Feb 19, 2018

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: {}
};

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
}
reflectionMap.instance[key] = Reflect.getOwnMetadataKeys(proto, key);
// hooks
if ($internalHooks.indexOf(key) > -1) {
options[key] = proto[key]
Expand Down Expand Up @@ -69,7 +77,8 @@ export function componentFactory (
: Vue
const Extended = Super.extend(options)

forwardStaticMembers(Extended, Component, Super)
forwardStaticMembersAndCollectReflection(Extended, Component, Super, reflectionMap)
copyReflectionMetadata(Component, Extended, reflectionMap)
Copy link
Member

@ktsn ktsn Feb 19, 2018

Choose a reason for hiding this comment

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

I think we don't need this if we directly forward metadata and remove reflectionMap.


return Extended
}
Expand All @@ -93,14 +102,21 @@ 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
}

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
29 changes: 29 additions & 0 deletions src/reflect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { VueConstructor } from 'vue'
import 'reflect-metadata'

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

export type ReflectionMap = {
instance: StringToArrayMap,
static: StringToArrayMap
}

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

function shallowCopy(from: VueConstructor, to: VueConstructor, propertyKeys: StringToArrayMap) {
for (const propertyKey in propertyKeys) {
propertyKeys[propertyKey].forEach((metadataKey) => {
const metadata = Reflect.getOwnMetadata(metadataKey, from, propertyKey)
Reflect.defineMetadata(metadataKey, metadata, to, propertyKey)
})
}
}
20 changes: 20 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,23 @@ describe('vue-class-component', () => {
expect(vm.valueA).to.equal('hi')
expect(vm.valueB).to.equal(456)
})

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

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

@Reflect.metadata('worksAccessor', true)
get testAccessor(): boolean { return true; }
set testAccessor(value: boolean) { void(value) }
}

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);
})
})