Support core-js/features/reflect?

[KeithGillette]

Is your feature request related to a problem? Please describe.
Our project uses the Reflect polyfill provided in core-js since core-js provides a range of other useful polyfills. However, when we attempt to use type-graphql with core-js/features/reflect, we get the following error: ReflectMetadataMissingError: Looks like you've forgot to provide experimental metadata API polyfill. Please read the installation instruction for more details.

Describe the solution you'd like
Update the type-graphql check for a Reflect polyfill to support the core-js implementation as well as reflect-metadata.

[MichalLytek]

As you can see, TypeGraphQL only checks with a duck-typing approach if the Reflect polyfill is provided:

type-graphql/src/metadata/utils.ts

Lines 60 to 68 in 9555a94

	export function ensureReflectMetadataExists() {
	if (
	typeof Reflect !== "object" ||
	typeof Reflect.decorate !== "function" ||
	typeof Reflect.metadata !== "function"
	) {
	throw new ReflectMetadataMissingError();
	}
	}

So something must be wrong with your setup as TypeGraphQL does not require reflect-metadata directly.

[KeithGillette]

Thanks for the fast reply, @MichalLytek. It looks like the second test is triggering the failure with core-js, as if I add the following to our main.ts entry point:

import 'core-js/features/reflect/';
console.log(typeof Reflect !== 'object' ); // false
console.log(typeof Reflect.decorate !== 'function'); // true
console.log(typeof Reflect.metadata !== 'function'); // false

While reflect-metadata defines Reflect.decorate, neither core-js nor reflect-metadata list it in their API documentation. As far as I can surmise, Reflect.decorate is not part of the ECMAScript proposal but an internal implementation detail of the reflect-metadata polyfill.

[MichalLytek]

Correct me if I'm wrong, but that's what TypeScript compiler requires in order to properly transpile decorators:

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};

[KeithGillette]

I don’t know enough about TypeScript internals to say but it does seem to be the case. However, the code snippet you quote includes a fall-back if the test for Reflect.decorate fails, I’m assuming because most projects don’t import a Reflect polyfill at all since runtime reflection is not needed. Isn’t the question whether any of the type-graphql code calls Reflect.decorate? A quick repository search didn’t turn up any calls, leading me to believe the existence test for that particular method is superfluous for this library, but perhaps I’m missing something.

[MichalLytek]

TypeGraphQL itself indeed does not call Reflect.decorate. The runtime check is there because a lot of newcomers don't read the docs and forgot to install deps and configure TS properly 😛

I don't know, maybe we can remove the check for Reflect.decorate === "function", leaving only the metadata one... 🤔

[KeithGillette]

Yes, having a runtime check to produce a friendly error message makes sense. I think that removing the check for Reflect.decorate would allow using any polyfill that only implements the public Reflect API proposal.

[MichalLytek]

Closing via 015939d 🔒