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.

Sign up for GitHub

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

Jump to bottom

Add typescript information for core-js-builder #1159

Closed
wants to merge 0 commits into from
Closed

Add typescript information for core-js-builder #1159

wants to merge 0 commits into from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jan 3, 2023

Add declarations for core-js-builder so it can be used from within TypeScript projects.

@zloirock
Copy link
Owner

zloirock commented Jan 3, 2023

Thanks, that makes sense.

@susnux
Copy link
Contributor Author

susnux commented Jan 3, 2023

Thank you for the feedback @zloirock ! I resolved the review notes and added the missing types.

@susnux susnux requested a review from zloirock January 3, 2023 20:08
@susnux susnux force-pushed the master branch 2 times, most recently from 099e3be to 80ea456 Compare January 3, 2023 20:33
@susnux susnux force-pushed the master branch 2 times, most recently from e8118a6 to 4d91a8f Compare January 3, 2023 22:11
@zloirock
Copy link
Owner

zloirock commented Jan 4, 2023

I made it stricter and fixed some issues.

@susnux
Copy link
Contributor Author

susnux commented Jan 4, 2023

I made it stricter and fixed some issues.

Have you removed the doc comments on purpose? I aded them to have information on what each option does within your IDE

@zloirock
Copy link
Owner

zloirock commented Jan 4, 2023

I'm not sure that it's something useful, but if you want - ok, let's return them.

@susnux
Copy link
Contributor Author

susnux commented Jan 4, 2023 •
edited
Loading

I'm not sure that it's something useful, but if you want - ok, let's return them.

I like explanations within the IDE, so you do not have to switch to your browser for the example within the README 😄

without with
without comment hovering an option hovering an option with comment

But I am also fine if you like the documentation in the README :)

@zloirock zloirock closed this Jan 22, 2023
@susnux
Copy link
Contributor Author

susnux commented Jan 22, 2023 •
edited
Loading

Any reason why you closed the PR?
master...susnux:core-js:master

@zloirock
Copy link
Owner

Sorry, I mistakenly forced pushed on rebase - and it seems since because of this PR is closed, I have no longer access to this branch. I'll move commits from this PR to a new branch.

@zloirock zloirock mentioned this pull request Jan 22, 2023
Merged
@zloirock
Copy link
Owner

#1163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zloirock zloirock zloirock left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@susnux @zloirock