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

test: add more object utils test + remove undefined value when use with invertKeysAndValues fnc #4764

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tiendn
Copy link
Contributor

@tiendn tiendn commented Oct 25, 2024

Description

Add filter undefined/null value when use with utils invertKeysAndValues function
Add more test at objects.test.ts

Drive-by changes

return Object.fromEntries(
    Object.entries(data)
+      .filter(([_, value]) => value !== undefined && value !== null) // Filter out undefined and null values
      .map(([key, value]) => [value, key]),
);

Related issues

None

Backward compatibility

Currently, nowhere call this invertKeysAndValues func

Testing

Yes, more tests.

Copy link

changeset-bot bot commented Oct 25, 2024

⚠️ No Changeset found

Latest commit: 07eb58f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

jmrossy
jmrossy previously approved these changes Oct 27, 2024
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

One optional comment but otherwise looks good! However the the build is failing. Please test your PRs locally by running yarn build && yarn test from the monorepo root

expect(result).to.eql({ '1': 'a', '2': 'b' });
});

it('invertKeysAndValues should return an empty object if the object is empty', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is optional but consider nesting another describe block under the outer describe block to organize the tests by their function. E.g. a describe('invertKeysAndValues', () => block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Please review again

@jmrossy jmrossy dismissed their stale review October 27, 2024 14:05

build is failing in yarn-test job

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.

2 participants