Lodash omit unset #25522

aj-r · 2018-05-04T03:17:08Z

Fixes #25382, fixes #25361, fixes #25365. See discussion in those bugs.

Also fixes incorrect FP types for the partial function.

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change. (Run with npm test.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes:
Increase the version number in the header if appropriate. N/A
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

Also added some better tests.

…yTyped

…Typed#25365,DefinitelyTyped#25382), fix partial

typescript-bot · 2018-05-04T03:22:26Z

@aj-r Thank you for submitting this PR!

🔔 @bczengel @chrootsu @stepancar @Ailrun @e-cloud @thorn0 @jtmthf @DomiR @typicode @niieani - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

typescript-bot · 2018-05-04T04:22:28Z

@aj-r The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

…yTyped into lodash-omit-unset

typescript-bot · 2018-05-05T22:22:56Z

@aj-r The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

typescript-bot · 2018-05-11T21:28:17Z

@aj-r I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

typescript-bot · 2018-05-12T13:24:46Z

@aj-r The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

aj-r

Unfortunately I needed to trim down the tests a bunch, otherwise the build fails due to lack of memory. This happens even without the changes from this PR, so I think it's caused by some change in TS 2.9.

Anyway, trimming down the tests make this PR a bit hard to read, so I've tried to highlight the significant changes to lodash-tests.ts. All changes in other files (common.d.ts, object.d.ts, fp.d.ts, generate-fp.ts`) are significant as well.

aj-r · 2018-05-12T15:22:03Z

types/lodash/common/object.d.ts

@@ -2860,15 +2860,23 @@ declare module "../index" {
         */
        omit<T extends AnyKindOfDictionary>(
            object: T | null | undefined,
-            ...paths: PropertyPath[]
+            ...paths: Array<Many<PropertyName>>


This type is technically the same as before, but I thought Many<PropertyName> better reflected how the value is being used.

aj-r · 2018-05-12T15:24:32Z

types/lodash/lodash-tests.ts

@@ -5396,7 +5161,7 @@ fp.now(); // $ExpectType number

    _.chain("abc").get(1); // $ExpectType LoDashExplicitWrapper<string>
    _.chain("abc").get(["0"], "_");
-    _.chain([42]).get(0, -1); // $ExpectType LoDashExplicitWrapper<number | undefined>
+    _.chain([42]).get(0, -1); // ExpectType LoDashExplicitWrapper<number>


This test was failing even on the default branch (probably due to a bug fix in TS2.9) so I had to disable the $ExpectType.

aj-r · 2018-05-12T15:24:58Z

types/lodash/lodash-tests.ts

@@ -5788,24 +5538,24 @@ fp.now(); // $ExpectType number
    const dictionary: _.Dictionary<AbcObject> = anything;
    const numericDictionary: _.NumericDictionary<AbcObject> = anything;

-    _.omit(obj, "a"); // $ExpectType Partial<AbcObject>
+    _.omit(obj, "a"); // ExpectType Pick<AbcObject, "b" | "c">   // NOTE: ExpectType disabled because it fails in TS2.4


Test the new omit overload

aj-r · 2018-05-12T15:26:06Z

types/lodash/lodash-tests.ts

-    fp.unset("a.b")(object); // $ExpectType boolean
-    fp.unset(["a", "b"])(object); // $ExpectType boolean
+
+    fp.unset("a.b", object); // $ExpectType { a: { b: string; c: boolean; }; }


FP's version of unset returns an object, not a boolean.

aj-r · 2018-05-12T15:27:30Z

types/lodash/lodash-tests.ts

-    _.propertyOf({}); // $ExpectType (path: Many<PropertyName>) => any
-    _({}).propertyOf(); // $ExpectType LoDashImplicitWrapper<(path: Many<PropertyName>) => any>
-    _.chain({}).propertyOf(); // $ExpectType LoDashExplicitWrapper<(path: Many<PropertyName>) => any>
+    _.propertyOf({}) as (path: _.Many<_.PropertyName>) => any;


These tests were also failing on the default branch - looks like TS 2.9 splits PropertyName into string | number | symbol, but earlier versions do not, so the $ExpectType does not work in this case.

typescript-bot · 2018-05-17T14:37:20Z

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

niieani

Looks fine to me.

rbuckton · 2018-05-18T20:48:49Z

Thanks for the contribution!

aj-r added 11 commits December 23, 2017 12:34

lodash: _.get should with numeric keys, too.

057754c

Also added some better tests.

lodash: add one more NumericDictionary overload for _.get

6e0532c

lodash: more reasonable index for _.get tests

5b606ca

Merge with upstream

7336172

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

ea5f3ab

…yTyped

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

6182842

…yTyped

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

3776df2

…yTyped

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

80c7a68

…yTyped

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

6729e6a

…yTyped

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

dd23c2e

…yTyped

lodash: improve omit/unset/mergeAll (DefinitelyTyped#25361,Definitely…

1e7f424

…Typed#25365,DefinitelyTyped#25382), fix partial

typescript-bot added this to Waiting for Reviewers in Pull Request Status Board May 4, 2018

typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback labels May 4, 2018

typescript-bot moved this from Waiting for Reviewers to Needs Author Attention in Pull Request Status Board May 4, 2018

typescript-bot added The Travis CI build failed and removed Awaiting reviewer feedback labels May 4, 2018

aj-r added 2 commits May 5, 2018 16:36

Merge branch 'master' of https://github.com/DefinitelyTyped/Definitel…

c600ecf

…yTyped into lodash-omit-unset

lodash: fix test failures

d45c6ed

typescript-bot moved this from Needs Author Attention to Waiting for Reviewers in Pull Request Status Board May 5, 2018

typescript-bot added Awaiting reviewer feedback and removed The Travis CI build failed labels May 5, 2018

typescript-bot moved this from Waiting for Reviewers to Needs Author Attention in Pull Request Status Board May 5, 2018

typescript-bot added The Travis CI build failed and removed Awaiting reviewer feedback labels May 5, 2018

Remove trivial files from tsconfig.json to avoid memory errors in tests

207c35e

Trim down tests to avoid memory error

79b4051

typescript-bot moved this from Needs Author Attention to Waiting for Reviewers in Pull Request Status Board May 12, 2018

typescript-bot added Awaiting reviewer feedback and removed The Travis CI build failed labels May 12, 2018

aj-r commented May 12, 2018

View reviewed changes

typescript-bot moved this from Waiting for Reviewers to Review in Pull Request Status Board May 17, 2018

typescript-bot added the Unmerged The author did not merge the PR when it was ready. label May 17, 2018

niieani approved these changes May 17, 2018

View reviewed changes

typescript-bot moved this from Review to Check and Merge in Pull Request Status Board May 17, 2018

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback Unmerged The author did not merge the PR when it was ready. labels May 17, 2018

rbuckton merged commit 011afe6 into DefinitelyTyped:master May 18, 2018

typescript-bot removed this from Check and Merge in Pull Request Status Board May 18, 2018

aj-r deleted the lodash-omit-unset branch July 26, 2018 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lodash omit unset #25522

Lodash omit unset #25522

aj-r commented May 4, 2018 •

edited

typescript-bot commented May 4, 2018 •

edited

typescript-bot commented May 4, 2018 •

edited

typescript-bot commented May 5, 2018 •

edited

typescript-bot commented May 11, 2018

typescript-bot commented May 12, 2018

aj-r left a comment

aj-r May 12, 2018

aj-r May 12, 2018

aj-r May 12, 2018

aj-r May 12, 2018

aj-r May 12, 2018

typescript-bot commented May 17, 2018 •

edited

niieani left a comment

rbuckton commented May 18, 2018

Lodash omit unset #25522

Lodash omit unset #25522

Conversation

aj-r commented May 4, 2018 • edited

typescript-bot commented May 4, 2018 • edited

typescript-bot commented May 4, 2018 • edited

typescript-bot commented May 5, 2018 • edited

typescript-bot commented May 11, 2018

typescript-bot commented May 12, 2018

aj-r left a comment

Choose a reason for hiding this comment

aj-r May 12, 2018

Choose a reason for hiding this comment

aj-r May 12, 2018

Choose a reason for hiding this comment

aj-r May 12, 2018

Choose a reason for hiding this comment

aj-r May 12, 2018

Choose a reason for hiding this comment

aj-r May 12, 2018

Choose a reason for hiding this comment

typescript-bot commented May 17, 2018 • edited

niieani left a comment

Choose a reason for hiding this comment

rbuckton commented May 18, 2018

aj-r commented May 4, 2018 •

edited

typescript-bot commented May 4, 2018 •

edited

typescript-bot commented May 4, 2018 •

edited

typescript-bot commented May 5, 2018 •

edited

typescript-bot commented May 17, 2018 •

edited