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

Wanting to contribute for Metadata Reflection and Decorators Support in TypeScript package #10784

Closed
ardatan opened this issue Nov 17, 2019 · 21 comments
Labels
confirmed We want to fix or implement it
Milestone

Comments

@ardatan
Copy link

ardatan commented Nov 17, 2019

When using typescript package in an Angular project, it doesn't have metadata reflection which Angular needs to have "experimentalDecorators": true and "emitDecoratorMetadata": true in compilerOptions of TS Compiler configuration which is missing in meteor-babel's preCompileTypescript. So this little change in meteor-babel wouldn't affect existing Meteor users but it would make Meteor support all those libraries using decorators and metadata reflection like Angular, TypeORM, TypeGraphQL, Inversify etc. I can create a PR for this issue if you want.
https://github.com/meteor/babel/blob/51258a2feb412131ec045a7fba613690d9854ad0/index.js#L149

But I think respecting tsconfig.json in the project would be better because there are some other compilerOptions, users may want to use now and later. If you want to force users in some options, you can throw an error during compilation process to make users change by themselves instead of overriding it which would be more consistent with the behavior of the IDE that respects tsconfig.json.

There is another option which is using babel-typescript(or respecting babelrc and add ts and tsx extensions to ecmascript package

extensions: ['js', 'jsx', 'mjs'],
) and disabling default flow plugins to prevent conflicts inside meteor-babel so there would be single compilation process handled in babel completely. And in this case not to break the existing behavior for flow users, we can remove flow plugin for ts files only in meteor-babel package.

What do you think? @benjamn

@ardatan ardatan changed the title Metadata Reflection and Decorators Support in TypeScript package Wanting to contribute for Metadata Reflection and Decorators Support in TypeScript package Nov 17, 2019
@benjamn
Copy link
Contributor

benjamn commented Nov 18, 2019

Here are my thoughts on the challenges of paying attention to tsconfig.json: #10610 (comment)

@ardatan
Copy link
Author

ardatan commented Nov 19, 2019

@benjamn The actual issue is lack of these two additional options prevents users of Angular, Inversify and other kind of libraries to benefit from this typescript package. However adding those two options wouldn't affect the existing users. I also mentioned to use babel-typescript with the existing babel structure of meteor-babel that can be extended by other babel plugins. So my point wasn't only about respecting tsconfig.json.
I created that PR that makes people able to use this package with Angular and similar libraries/frameworks.
meteor/babel#28

@MastroLindus
Copy link

@benjamn I have the feeling that only a very limited amount of tsconfig settings will impact meteor's build (stuff like module, targets, etc), and that not allowing users to specify the other flags will create a lot more problems than the confusion of having that small set forced by meteor.

Couldn't we simply for the moment use a "best effort" approach to respect the tsconfig flags, and discard those that influence meteor's behaviour?
I am not a fan of this approach either, but between this and not been able to specify any flag at all I consider the latter much worse.

@stale
Copy link

stale bot commented Dec 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Dec 23, 2019
@afrokick
Copy link
Contributor

not stale

@stale stale bot removed the stale-bot label Dec 24, 2019
@afrokick
Copy link
Contributor

Also we use resolveJsonModule with decorators.

@afrokick
Copy link
Contributor

afrokick commented Jan 16, 2020

@benjamn @filipenevola any news? It is a blocker for us to migrate from Adornis package to "native" typescript.

There is open PR to meteor-babel meteor/babel#28

@MastroLindus
Copy link

MastroLindus commented Feb 14, 2020

I am also blocked in migrating from Adornis to the typescript package until some flags in tsconfig.json are allowed: #10610 (comment)

In short I need at least

strict: true,
experimentalDecorators: true

to be allowed.

I really really really hope they get supported.
Honestly I would still prefer to allow people to specify what they want in their own tsconfig, and have Meteor make a best-effort approach of supporting them, but overriding the ones that can be problematic (output directories, modules, etc)

Edit:
ah, I was forgetting, another really important thing: output of compilation errors/warnings, relying on the IDE for that just doesn't work for us, too many cases of people not noticing an error because the ide didn't check it as it was not in an open editor, and so on. The main point of typescript of increasing security through compilation checks is just wasted if the compiler won't show errors in the output

@stale
Copy link

stale bot commented Mar 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Mar 18, 2020
@Urigo
Copy link
Contributor

Urigo commented Mar 18, 2020

no stale.

I believe @ardatan gave a couple of possible solutions, can someone decide and offer one of them?
#10784 (comment)

@stale stale bot removed the stale-bot label Mar 18, 2020
@stale
Copy link

stale bot commented Apr 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Apr 19, 2020
@Urigo
Copy link
Contributor

Urigo commented Apr 19, 2020

no stale.

@stale stale bot removed the stale-bot label Apr 19, 2020
@ruohki
Copy link
Contributor

ruohki commented May 20, 2020

is there a workaround until we can use that by default?

@ardatan
Copy link
Author

ardatan commented May 20, 2020

You can use barbatus:typescript package instead of official one.

@stale
Copy link

stale bot commented Jun 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Jun 21, 2020
@ardatan
Copy link
Author

ardatan commented Jun 21, 2020

no stale.

@stale stale bot removed the stale-bot label Jun 21, 2020
@filipenevola
Copy link
Collaborator

filipenevola commented Jun 24, 2020

Hi @ardatan I've talked with @perbergland about his typescript package as well (https://github.com/ref-app/meteor-typescript) and we should join efforts here to update the official typescript package in the best way for the community.

@zodern is also going to take a look in this package from Per to check if the performance could be improved.

We need to select a package to be the candidate for the new official package and then make PRs to update the actual official package.

Any thoughts on this?

@hexsprite and @leonardoventurini feel free to share your opinions as well.

@ardatan
Copy link
Author

ardatan commented Jun 24, 2020

@filipenevola I think it is fine. The most important point is to have enough flexibility to support user's tsconfig.json (at least decorators).

@ruohki
Copy link
Contributor

ruohki commented Aug 22, 2020

Does someone have a workaround to hack this into a installed meteor ~ those other typescript packages are just so slow or bug prone

@stale
Copy link

stale bot commented Oct 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Oct 31, 2020
@afrokick
Copy link
Contributor

afrokick commented Nov 1, 2020

1.12 should contains emitDecorator

@stale stale bot removed the stale-bot label Nov 1, 2020
@filipenevola filipenevola added confirmed We want to fix or implement it and removed pinned labels Nov 11, 2020
@filipenevola filipenevola added this to the Release 1.12 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed We want to fix or implement it
Projects
None yet
Development

No branches or pull requests

7 participants