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

[TypeScript] typegen creating output which conflicts with @typescript-eslint/ban-types #411

Open
simonflk opened this issue Jan 29, 2022 · 8 comments
Labels

Comments

@simonflk
Copy link

simonflk commented Jan 29, 2022

Description

Typegen output is offending @typescript-eslint/ban-types whose mission statement is

Some builtin types have aliases, some types are considered dangerous or harmful. It's often a good idea to ban certain types to help with consistency and safety.

-- README

Expected Result

Typegen output should not use incorrect types which are flagged by the @typescript-eslint/ban-types rule

Actual Result

The specific error is:

Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.e

And can occur on invokeSrcNameMap, eventsCausingServices, eventsCausingGuards, and eventsCausingDelays:

image

Reproduction

Typegen doesn't run on codesandbox, but the following machine will trigger the above error on [email protected]:

import { createModel } from 'xstate/lib/model';

export const testModel = createModel(
  {
    name: null as null | string,
  },
  {
    events: {
      setName: (name: string) => ({
        name,
      }),
      reset: () => ({}),
    },
  },
);

export const surveyFormMachine = testModel.createMachine({
  tsTypes: {},
  id: 'test',
  initial: 'idle',
  context: testModel.initialContext,
  states: {
    idle: {},
  },
  on: {
    reset: {
      actions: 'resetContext',
    },
    setName: {
      actions: 'setName',
    },
  },
});

Additional context

XState 4.29.0
statelyai.stately-vscode 1.5.6

@simonflk
Copy link
Author

I'm currently working around this with an override in my eslint.json to disable that rule for *.typegen.ts

@Andarist
Copy link
Member

@mattpocock we can probably just output never at those locations instead of {} - but it's best to double-check that and add those to all existing type gen tests in this repo. Maybe we could even just skip those keys if they are empty~? That could be even better as we'd save some parsing time.

@mattpocock
Copy link
Contributor

Yes, although applying any linting whatsoever to generated files is probably not a great idea. When we release the CLI, we'll even be able to gitignore these assets and generate them on CI.

This is a cheap enough change to make, though.

@horacioh
Copy link

When we release the CLI, we'll even be able to gitignore these assets and generate them on CI.

@mattpocock Is there any example on how to do this on GH Actions? thanks in advance!

@mattpocock
Copy link
Contributor

@horacioh Yes, see the CLI docs:

https://xstate.js.org/docs/packages/xstate-cli/#xstate-cli

@horacioh
Copy link

YES!! I saw it and forgot to mention it here ;)

@nostrorom
Copy link

nostrorom commented Jan 13, 2023

HI all !

I'm having the same issue - toying around with a pretty simple machine that typegens this:

export interface Typegen0 {
  "@@xstate/typegen": true;
  internalEvents: {
    "xstate.init": { type: "xstate.init" };
    "xstate.stop": { type: "xstate.stop" };
  };
  invokeSrcNameMap: {};
  missingImplementations: {
    actions: never;
    delays: never;
    guards: never;
    services: never;
  };
  eventsCausingActions: {
    logEntry: "PLAY" | "STOP";
    logExit: "PAUSE" | "STOP" | "xstate.stop";
    logPaused: "PAUSE";
  };
  eventsCausingDelays: {};
  eventsCausingGuards: {};
  eventsCausingServices: {};
  matchesStates: "paused" | "playing" | "stopped";
  tags: never;
}

And the warnings as similar to what is described above

Don't use {} as a type. {} actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want 'object' instead.
- If you want a type meaning "any value", you probably want 'unknown' instead.
- If you want a type meaning "empty object", you probably want 'Record<string, never>' instead. 
eslint(@typescript-eslint/ban-types)

So what is the recommended way to handle this ? Add typegen files to eslintignore ?

Thanks

@Andarist
Copy link
Member

So what is the recommended way to handle this ? Add typegen files to eslintignore ?

Yes, this is a reasonable approach.

I will probably look in the future for alternative approaches for this but the mentioned ESLint rule shouldn't always be blindly trusted anyway. There are some cases for which {} is a better choice than all of the alternatives mentioned in this warning. So I'm not quite treating this as a high-priority issue.

@davidkpiano davidkpiano transferred this issue from statelyai/xstate Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants