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

NodeNext compatibility #20

Closed
marcoreni opened this issue Nov 23, 2022 · 9 comments · Fixed by #21
Closed

NodeNext compatibility #20

marcoreni opened this issue Nov 23, 2022 · 9 comments · Fixed by #21

Comments

@marcoreni
Copy link

When using this plugin with moduleResolution: nodeNext, we encounter the same error as fastify/fastify-cors#223 .

@smartiniOnGitHub
Copy link
Owner

smartiniOnGitHub commented Nov 23, 2022

Hi, thanks for the issue, but could you give me a minimal example to reproduce the error or is it the same of the linked issue ?
With the workaround seen there, does it work ?
I see there is a related bug opened in Fastify, see here; let's check after it has been resolved and see if something can be fixed/improved even in this plugin.
Thanks a lot.

@smartiniOnGitHub
Copy link
Owner

smartiniOnGitHub commented Nov 23, 2022

ok perfect I'll take a look during next days and tell you something ...
Thanks for now. Bye

@smartiniOnGitHub
Copy link
Owner

smartiniOnGitHub commented Nov 28, 2022

Hi, now I better understand the problem; but I have a little TypeScript knowledge so I'll need some time for the fix.
Anyway suggestions/PR are welcome in the meantime. Thanks for now. Bye

@smartiniOnGitHub
Copy link
Owner

Sorry this is longer than expected ... anyway in the repo you can see that I started the work on it (bump next release that will be 4.3.0 and upgrading all dependencies to latest, to be sure that all will be good); I'll tell more soon. Bye

@smartiniOnGitHub
Copy link
Owner

Update: changes almost done and all seems to work; now I only have to let the linter handle the new code in the right way.
Soon I'll raise a PR so if needed you can take a look and maybe do some tests before the merge.

@smartiniOnGitHub
Copy link
Owner

@marcoreni Hi, PR with code changes opened, please take a look at changes; I tested in another sample project that uses TypeScript with NodeNext and all should be fine. If not tell me, otherwise in a few days I'll merge this and prepare for a next plugin release. Thanks. Bye

@marcoreni
Copy link
Author

Hei @smartiniOnGitHub , I did a quick&dirty check by replacing the .d.ts file in the package with the one from the PR and it seems to work.

@smartiniOnGitHub
Copy link
Owner

Release 4.3.0 of the plugin with the fix just published. Bye

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants