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

Untyped JS module is not assignable to type even though it satisfies it #57460

Closed
bradzacher opened this issue Feb 21, 2024 · 20 comments Β· Fixed by #57467
Closed

Untyped JS module is not assignable to type even though it satisfies it #57460

bradzacher opened this issue Feb 21, 2024 · 20 comments Β· Fixed by #57467
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@bradzacher
Copy link
Contributor

bradzacher commented Feb 21, 2024

πŸ”Ž Search Terms

Untyped JS module is not assignable to type even though it satisfies it, "Object literal may only specify known properties"

⏯ Playground Link

https://github.com/bradzacher/bug-repros/tree/ts-js-issue-huh

πŸ’» Code

import eslintReact from 'eslint-plugin-react/configs/recommended.js';

(eslintReact satisfies {
  plugins: {
      react: {
          rules: any;
      };
  };
});

πŸ™ Actual behavior

There should be no errors because the type
{ plugins: { react: { deprecatedRules: any; rules: any; }; }; }
is assignable to
{ plugins: { react: { rules: any; }; }; }

πŸ™‚ Expected behavior

Type '{ plugins: { react: { deprecatedRules: any; rules: any; }; }; rules: any; readonly languageOptions: any; } & { languageOptions: any; rules: { 'react/display-name': number; 'react/jsx-key': number; 'react/jsx-no-comment-textnodes': number; ... 18 more ...; 'react/require-render-return': number; }; }' does not satisfy the expected type '{ plugins: { react: { rules: any; }; }; }'.
  The types of 'plugins.react' are incompatible between these types.
    Object literal may only specify known properties, and 'deprecatedRules' does not exist in type '{ rules: any; }'

Additional information about the issue

This issue is particularly weird because it only occurs in a JS file!

For more context this is the original bug report typescript-eslint/typescript-eslint#8522
And the original repro from the issue (the usecase we really want to fix): https://github.com/printfn/tseslint-react-repro

We have built support for ESLint flat configs via our typescript-eslint package and it exports a function to allow typechecking of the config array. So the repo looks like this

// @filename eslint.config.js


// @ts-check

import tseslint from 'typescript-eslint';
import eslintReact from 'eslint-plugin-react/configs/recommended.js';

export default tseslint.config(
	eslintReact, // type error is on this line
);

Best I can tell it looks like TS is treating the eslintReact variable the same way it would as an inline object expression - hence it's erroring saying "too many properties".

@Andarist
Copy link
Contributor

@bradzacher
Copy link
Contributor Author

oops sorry made it public

@Andarist
Copy link
Contributor

the repro doesn't contain a concrete TS version and a tsconfig.json :P

@bradzacher
Copy link
Contributor Author

bradzacher commented Feb 21, 2024

Yeah I know. The latter one is intentional.
Users don't setup an eslint.config.js and cover it with a tsconfig - it just uses the vscode defaults

As for the TS version - I doubt it matters? I was just using whatever is installed in the latest vscode version.

In the issue I linked a bigger repro if you want some more stuff (https://github.com/printfn/tseslint-react-repro). I was just trying to be as isolated as possible.

@Andarist
Copy link
Contributor

There is some difference between my default and your default setups then because I don't get this error but rather this at the import statement:

Module '"/repros/bug-repros/foo"' has no default export.ts(1192)

We have to use different module resolutions for some reason.

@bradzacher
Copy link
Contributor Author

sod it, sorry @Andarist - I forgot to push a fix.
The import in foo.ts should be from eslint-plugin-react/configs/recommended.js (I just updated the repo)

@Andarist
Copy link
Contributor

I still get a different error:

Module '"/repros/bug-repros/node_modules/eslint-plugin-react/configs/recommended"' can only be default-imported using the 'esModuleInterop' flagts(1259)
recommended.js(5, 1): This module is declared with 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.

@Andarist
Copy link
Contributor

Andarist commented Feb 21, 2024

Or this one:

Could not find a declaration file for module 'eslint-plugin-react/configs/recommended.js'. '/repros/bug-repros/node_modules/eslint-plugin-react/configs/recommended.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/eslint-plugin-react` if it exists or add a new declaration (.d.ts) file containing `declare module 'eslint-plugin-react/configs/recommended.js';`ts(7016)

I'm not sure what makes VS Code change its mind about which error is supposed to be displayed πŸ˜‰ This might depend on skipLibCheck, allowJs and alike. But since there is no tsconfig here I can't really control the desired behavior

EDIT:// I think it might depend on node_modules/eslint-plugin-react/configs/recommended.js being open in a different tab or not.

@bradzacher
Copy link
Contributor Author

bradzacher commented Feb 21, 2024

Maybe there's some vscode settings that impact this?
Have you changed any of the defaults?

I spent a while playing around with a tsconfig.json but I couldn't make it repro with any combo of tsconfig settings I tried which is why I omitted it.

Can you give this repro a go? https://github.com/printfn/tseslint-react-repro
Just install and open the eslint.config.js.
If it doesn't repro there then you defs have tweaked some settings.

@Andarist
Copy link
Contributor

No luck either even after disabling extensions. I downloaded the insiders version of VS Code and I managed to repro it there

@Andarist
Copy link
Contributor

This should fix this incorrectly reported error: #57467 . However, it still won't typecheck in your case because this property from the source isn't compatible with your target: https://github.com/jsx-eslint/eslint-plugin-react/blob/36e791de784e7fa1edd5b6d232fbcaf2dd7cf9bf/configs/all.js#L28

Its type gets computed as { [x: string]: number; [x: number]: number; [x: symbol]: number; }. See the error reported here: TS playground

@bradzacher
Copy link
Contributor Author

Its type gets computed as { [x: string]: number; [x: number]: number; [x: symbol]: number; }.

If I locally modify my node_modules and comment out the line that's currently erroring then I get no more errors in my IDE (i.e. I can't repro that wide signature).

Unless I'm misunderstanding and you're saying that after your fix the record is inferred as that wide type?

@Andarist
Copy link
Contributor

If I locally modify my node_modules and comment out the line that's currently erroring then I get no more errors in my IDE (i.e. I can't repro that wide signature).

When doing just that (without applying my patch in tsserver, just removing that single line with deprecatedRules property) I get the same error as the one that I get with my tsserver patch applied (the one showcased on the TS playground above).

Could you try hovering over activeRulesConfig here? What type is displayed by the quick info? Is it { [x: string]: number; [x: number]: number; [x: symbol]: number; }?

@bradzacher
Copy link
Contributor Author

Note: filed jsx-eslint/eslint-plugin-react#3694 to add some minimal JSDoc annotations which will short-circuit the TS inference and fix things.

@bradzacher
Copy link
Contributor Author

Could you try hovering over activeRulesConfig here? What type is displayed by the quick info? Is it { [x: string]: number; [x: number]: number; [x: symbol]: number; }?

image

It just shows any for me.

@Andarist
Copy link
Contributor

πŸ˜…
`quick info showing { [x: string]: number; [x: number]: number; [x: symbol]: number; }`

Could you try applying my patch to see what happens with it?

@bradzacher
Copy link
Contributor Author

bradzacher commented Feb 21, 2024

still any 😨

the error disappeared though so your patch definitely was applied.

I think you're getting hit by automatic type acquisition and TS is automatically pulling in @types/object.fromentries.
If I manually install that then I get the same behaviour.

Essentially without @types/object.fromentries the object.fromentries package is too complex for TS to infer types for -- so the fromEntries call returns any.
With it TS understands the return type and thus propagates that up the call stack and infers that final (incorrect) type.

The JSDoc annotations I added in jsx-eslint/eslint-plugin-react#3695 will short-circuit TS's inference here so that ATA won't change the signature

@Andarist
Copy link
Contributor

I think you're getting hit by automatic type acquisition and TS is automatically pulling in @types/object.fromentries.

Yeah, that's probably it. Thanks for looking into this.

The JSDoc annotations I added in jsx-eslint/eslint-plugin-react#3695 will short-circuit TS's inference here so that ATA won't change the signature

IIUC ATA might impact activeRulesConfig's type and you have left that untyped so it won't fix your issue completely. The added annotations there might help with the bug fixed by #57467 though πŸ‘

@bradzacher
Copy link
Contributor Author

IIUC ATA might impact activeRulesConfig's type and you have left that untyped so it won't fix your issue completely

const activeRulesConfig = configureAsError(activeRules); -- I typed configureAsError so the type flows into activeRulesConfig so it should fix things!


"typescript.disableAutomaticTypeAcquisition": true
I disabled ATA a long time ago because I kept getting bit by it auto-grabbing @types/eslint when I didn't want it to.

@Andarist
Copy link
Contributor

const activeRulesConfig = configureAsError(activeRules)

ah cool, I somehow missed that πŸ‘

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Feb 22, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
3 participants