Skip to content

nodenext compatibility, correct typings #22

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

Merged
merged 7 commits into from
Dec 9, 2022
Merged

nodenext compatibility, correct typings #22

merged 7 commits into from
Dec 9, 2022

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Dec 5, 2022

Part of Migration of the infamous triplet to next level, Nodenext support

Checklist

@Uzlopak Uzlopak requested review from Eomm and climba03003 December 5, 2022 10:00
@mcollina
Copy link
Member

mcollina commented Dec 5, 2022

CI seems failing

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Dec 5, 2022

The original typings are wrong. Currently checking

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Dec 5, 2022

Ok, fixed the typings. They were from the beginning wrong. So I assume, nobody uses fast-json-stringify-compiler in a typescript environment.

I had to change the purpose of some typings, which means, that it could be a breaking change. But on the other hand, as mentioned before, nobody complained about the fact that the typings were wrong, so probably nobody will complain anyway.

@Uzlopak Uzlopak changed the title nodenext compatibility nodenext compatibility, correct typings Dec 5, 2022
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina
Copy link
Member

mcollina commented Dec 5, 2022

Let's wait for @Eomm before landing this one.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Dec 5, 2022

I agree.

@Uzlopak Uzlopak merged commit b91042c into main Dec 9, 2022
@Uzlopak Uzlopak deleted the nodenext branch December 9, 2022 13:39
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 this pull request may close these issues.

3 participants