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

lodash.merge can Bypass Type Check in Typescript #48950

Closed
dsdashun opened this issue May 4, 2022 · 3 comments
Closed

lodash.merge can Bypass Type Check in Typescript #48950

dsdashun opened this issue May 4, 2022 · 3 comments
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@dsdashun
Copy link

dsdashun commented May 4, 2022

Bug Report

πŸ”Ž Search Terms

lodash+merge

πŸ•— Version & Regression Information

4.x ( haven't tried any older versions )

  • I was unable to test this on prior versions because it failed to compile my node_modules folder

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

import * as _ from "lodash";

interface MyConfig {
  main: MainConfig;
  opt?: OptionalConfig;
}

interface MainConfig {
  logLevel: string;
}

interface OptionalConfig {
  boolKey: boolean;
  intKey1?: number;
}

let myConfigObj: MyConfig = {
  main: {
    logLevel: "info",
  },
};

let mergedConfigObj: MyConfig;

/*
mergedConfigObj = _.merge({}, myConfigObj)
mergedConfigObj.opt = 234;    // this will report error on compilation, which is expected
*/

mergedConfigObj = _.merge({}, myConfigObj, {
  opt: 123,
}); // this is not expected: it means we can use lodash.merge to assign an object with wrong type!
console.log(mergedConfigObj);    // result: { main: { logLevel: 'info' }, opt: 123 }

πŸ™ Actual behavior

Surprisingly, this code can compile without errors. This means we can use lodash.merge to assign some optional fields with the wrong type, thus bypassing the type check in typescript

πŸ™‚ Expected behavior

I expect the code will compile error, indicating that the opt field cannot assign with a number type

@jcalz
Copy link
Contributor

jcalz commented May 4, 2022

I'm not sure why this is being filed against TS instead of against lodash, whose typings are (I think) in DefinitelyTyped/DefinitelyTyped.

Still, this is similar enough in spirit to the various issues that have been filed against Object.assign() that we could probably say this is a duplicate of those, e.g., #31982. Intersections are a "best available compromise" for the results of this sort of merge; you could write something quite complicated to try dealing with conflicting properties, but there will always be edge cases.

@IllusionMH
Copy link
Contributor

IllusionMH commented May 4, 2022

UPD. Reread #31982 and it has all answers why reduced case with intersection doesn't work as expected.

@andrewbranch andrewbranch added the External Relates to another program, environment, or user action which we cannot control. label May 4, 2022
@typescript-bot
Copy link
Collaborator

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

5 participants