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 NonEmptyObject type #623

Merged
merged 11 commits into from
Aug 25, 2023
Merged

Add NonEmptyObject type #623

merged 11 commits into from
Aug 25, 2023

Conversation

kkmuffme
Copy link
Contributor

Close #621

@kkmuffme
Copy link
Contributor Author

kkmuffme commented May 21, 2023 •
edited

@sindresorhus the failing test reports an error in an empty line? Could you please pinpoint me at what the issue is?

Looked at the .ts instead of the .d.ts by accident.

@kkmuffme kkmuffme marked this pull request as ready for review May 23, 2023 14:26
@sindresorhus
Copy link
Owner

The diff view shows the error inline. You should also always run npm test locally before pushing, which also shows the error description.

@sindresorhus sindresorhus mentioned this pull request Aug 8, 2023
Closed
@sindresorhus
Copy link
Owner

Closing for lack of response.

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Aug 8, 2023

It's done this second :D

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Aug 8, 2023

@sindresorhus ha ha, I just fixed and finished it in exactly the second you closed it. Can you reopen & merge it please.

@sindresorhus sindresorhus reopened this Aug 18, 2023
sindresorhus
sindresorhus requested changes Aug 18, 2023
View reviewed changes
source/non-empty-object.d.ts Outdated Show resolved Hide resolved
source/non-empty-object.d.ts Outdated Show resolved Hide resolved
source/non-empty-object.d.ts Outdated Show resolved Hide resolved
source/non-empty-object.d.ts Show resolved Hide resolved
test-d/non-empty-object.ts Outdated Show resolved Hide resolved
test-d/non-empty-object.ts Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

You need to fix the merge conflicts.

@sindresorhus
Copy link
Owner

Tests are failing.

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Aug 18, 2023 •
edited

Yeah, I decided to change the type as I realized it was behaving unexpectedly.

However now, the failing tests are not clear. The test has exactly the type I see in playground:
https://www.typescriptlang.org/play?target=99&module=1&jsx=0#code/JYWwDg9gTgLgBDAnmApnA3gCQIYGcBKKAjgK7BQoAmA0iorgDRyGnkoCCMAMinjAPIA7FEwCSuAKKlsAGwC+cAGZQIIOAHIkqALSKUuGOoDcAKBMoAHpFgJkaAHIRBE8En4AjAFYoAxjAA8ACpwljAogpS4cBBevjAAfHAAvHA4BMRkFDR0uEGJoeGRCFAkaAD8cMEAXMwZbJw8fEIoQUwA1nQQipXxpiZaaIH6MIF2AEzJGCYAkNhlNQZQwIIA5qbT7vNw7hAQMryCpnJ9lL4y2BRwPk4GCMNjNY7Oroge3n5Bw6OoY71mA3AJFZsBFvi1AokUsEChEogAKAB0SIuK1wNWWeigcHYAEpkokMSgsfgTHA4BVEcioKiakCwCDKGD-Ox4nikok6Qymfh4qS4DVoRYwrC4ISsfw+RV0HAANrUUWCOAdRBdOD8AC6NX4cvVcDkfJqwgAbkS+v07IDgREqJNOaC7P4BqqwgZfqYAVISLJJuJPbJ-OhsAsYEtVkZtlsdnsDnAAD5wEjWxTLKjhhTxwNbRbLFZxhNJlOUcPuGpR-YgtNMO2Mh1PFxgNyxD5DAxg37xP5mExAA issue is unrelated to this PR

@kkmuffme
Copy link
Contributor Author

@sindresorhus all tests pass, please review again and merge it

@sindresorhus sindresorhus merged commit 98bb74d into sindresorhus:main Aug 25, 2023
5 checks passed
@sindresorhus
Copy link
Owner

Thanks ;)

@kkmuffme kkmuffme deleted the type-non-empty-object branch August 29, 2023 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sindresorhus sindresorhus sindresorhus requested changes

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

Successfully merging this pull request may close these issues.

NonEmptyObject
2 participants
@kkmuffme @sindresorhus