Skip to content
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.

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

reflect-metadata specific version needed? #563

Closed
grofit opened this issue Apr 13, 2018 · 5 comments
Closed

reflect-metadata specific version needed? #563

grofit opened this issue Apr 13, 2018 · 5 comments

Comments

@grofit
Copy link

grofit commented Apr 13, 2018

I'm submitting a request

Current behavior

NestJs currently seems to depend on a specific version of reflect-metadata (0.1.10) and will complain in NPM unless you resolve this SPECIFIC version, I tried higher versions and it complained it needed that specific one.

Expected behavior

It should ideally not need any specific polyfill for Reflect.getMetadata etc it should just expect the consumer to provide one (until nodejs etc) doesn't need it polyfilled.

Currently to my knowledge there are 2 available polyfills:

  • core-js
  • reflect-metadata

Problem here is if you were to have both of them in a project your metadata gets wiped as each polyfill is replacing each others polyfill (this may be fixed now but was an issue a few months back).

Minimal reproduction of the problem with instructions

  • Make a new project
  • Add restjs dependencies
  • See error for missing reflect-metadata 0.1.10
  • install 0.1.12 version of reflect-metadata
  • See error for missing reflect-metadata 0.1.10

What is the motivation / use case for changing the behavior?

It just seemed odd that it should force you to use a specific version of a polyfill, and in some cases people may want to use core-js which would probably cause decorators that use metadata to blow up.

Recommended solution?

I would remove the dependency on reflect-metadata entirely, and just include a devDependency for @types/reflect-metadata, that will allow you to write TS code that uses metadata, then you require the consumer to provide a relevant polyfill (as listed above).

This is done on one of my other libs:

@ith
Copy link

ith commented Apr 14, 2018

Upvote for this. I have the exact same issue

@weeco
Copy link

weeco commented Apr 19, 2018

Side note: There are in general a lot outdated dependencies in the package.json. Maybe use greenkeeper to keep them up to date?

@kamilmysliwiec
Copy link
Member

Hey @grofit,
Thanks for reporting! This issue is fixed in v5.0.0 (currently v5.0.0-beta.2).

@amitport
Copy link
Contributor

@kamilmysliwiec the dependency seems to be still forced in the bundles
(

"reflect-metadata": "^0.1.12",
)

@lock
Copy link

lock bot commented Sep 25, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants