check reflectionIsSupported in decorator runtime

[vitalishapovalov]

In this commit, reflectionIsSupported has been refactored from (Reflect && Reflect.defineMetadata) !== undefined to typeof Reflect !== undefined && Reflect.defineMetadata.

But also, from decorator runtime (function) check to one-time check on import (var). Which caused a small issue:

I've faced a situation, where one of my dependencies brings reflect-metadata as it's own dependency, and being imported after vue-class-component package.
When decorating is applied, metadata already treated as unsupported and metadata is lost.

The only way to solve this is to import reflect-metadata before our main App module (in other words, before decorated are applied).

But in my case, I don't need reflect-metadata, except for this dependency. Even more, this dependency is used only during development and not imported in production.

This PR can solve this issue.

[ktsn]

But in my case, I don't need reflect-metadata, except for this dependency. Even more, this dependency is used only during development and not imported in production.

In that case, why is it a problem for you that vue-class-component does not handle meta data? If you want vue-class-component to handle it, you can simply add import statement for reflect-metadata before importing vue-class-component, I guess.

[vitalishapovalov]

But in my case, I don't need reflect-metadata, except for this dependency. Even more, this dependency is used only during development and not imported in production.

In that case, why is it a problem for you that vue-class-component does not handle meta data? If you want vue-class-component to handle it, you can simply add import statement for reflect-metadata before importing vue-class-component, I guess.

Yeah, you're almost absolutely right!

The only way to solve this is to import reflect-metadata before our main App module (in other words, before decorated are applied).

vue-class-component is imported in each component (@Component).
So, to solve this import-order issue with this approach, we would need to import all of the conflicting imports in 1 place (for the first time), before any other imports (that's not what what you usually do with such libs as vue-class-component in your App).
I'm not even talking about the tree-shaking, which will exclude some libs, and you will need to additionally configure it.

The case I'm talking about is when u need to decorate a single component, in one place, temporary (for some developments staff, tests, logs), to check something, and you dont have (and dont need) reflect-metadata lib.
And to do this in our case, we should to staff I've described above (with imports).

OR, we can process reflect dynamically, as I've proposed in this PR.
Maybe this change brings some issues that I can't see?

[ktsn]

You can import the polyfill as webpack (or any bundler) entry script or adding import statement in the very beginning in your entry script. You don't need to import it by every appearance of vue-class-component import. And I see it is a common way to load polyfill in my experience.

[vitalishapovalov]

I'm talking about the case, where I don't need this polyfill in production and I don't want to edit my webpack config just because of reflect metadata. I need this metadata polyfill only when my own decorator is used, and it is used like:

1. Add decorator
2. See console log
3. Remove decorator

With your proposition it looks like that:

1. Add reflect-metadata polyfill / entrypoint import
2. Add decorator
3. See console log
4. Remove decorator
5. Remove reflect-metadata polyfill / entrypoint import

Or we can make dynamic configuration with tree-shaking etc., which is obviously an overhead for a simple stuff I'm talking about.

My decorator bring this dep with himself, but it SHOULDN'T be imported in entry point or moved to separate entrypoint, as long as it dependencies, like reflect metadata (to keep it simple).

[ktsn]

What do you mean by Remove decorator and Remove reflect-metadata? I'm still not sure what you mean and your use case...

In any cases, if you want to tree-shake reflect-metadata, it doesn't matter whether you have a third-party lib which includes it as your problem still the case without it. I don't think this change solves your problem.

If you really want to split your chunks even with reflect-metadata, it would be better to create bridge file to handle it.

// ./class-component.js
import 'reflect-metadata'
import Component from 'vue-class-component'

export default Component

import Vue from 'vue'
import Component from '@/class-component'

@Component
export default MyComp extends Vue {
}

[vitalishapovalov]

As an example: I have a tool, which does nothing except a dozens of logs. It is very specific and rarely-needed. It is used in following way:

1. We have a component. It is not a top-level component. We don't have reflect-metadata lib installed.

// /components/.../DeepComponent.vue
import Vue from 'vue';
import Component from 'vue-class-component';

@Component
export default class DeepComponent extends Vue {
}

2. Now, we want to use @MyDecorator which will produce some logs and depends on metadata provided by @Component. We want to remove it after logging is done.

a) Expected:

// /components/.../DeepComponent.vue
import Vue from 'vue';
import Component from 'vue-class-component';
import MyDecorator from 'my-lib';

@MyDecorator
@Component
export default class DeepComponent extends Vue {
}

b) Actual:
First, install polyfill, as it was transitive dependency before.

npm install reflect-metadata

Then, import it on a top-level module

// /components/App.vue
import 'reflect-metadata';

@Component
export default class App extends Vue {
}

And only after, use it as expected.
There is a high possibility that this MyDecorator will be used 1-2 times during development max.
And when you don't need it anymore, you need to revert all the changes and uninstall polyfills, that are don't needed anymore.

Also, lib should have notification, something like

When using with vue-class-component lib, make sure you've imported reflect-metadata before your main App module.

[ktsn]

Ah I finally get what you mean. Thank you for clarifying it!

Also, lib should have notification, something like

No it shouldn't, as it is totally optional.

[vue-bot]

Thanks again! 💚

[vitalishapovalov]

Thanks!