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

Type checking sensitive to loading order #45859

Closed
Andarist opened this issue Sep 14, 2021 · 4 comments
Closed

Type checking sensitive to loading order #45859

Andarist opened this issue Sep 14, 2021 · 4 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@Andarist
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

sensitive, order, ordering, loading

πŸ•— Version & Regression Information

This doesn't work correctly with 4.3 and 4.4

⏯ Repository Link

https://github.com/statelyai/xstate-viz/tree/58b255c0769f2e9e0c292864057d83f50fe1b31b/src

(it's not possible to create a playground for this since the repro involves multiple files)

πŸ’» Code

This is not yet a minimal repro case but I have spent a lot of time on creating a minimal consuming/application code that triggers the problem. If the TS team confirms that this is an issue I might try to reduce this further down but maybe this is a duplicate of some issue that I couldn't find so it might turn out that spending more hours on this wouldn't be worth it.

// App.tsx
import { StateNode } from 'xstate';

export function toDirectedGraph(stateNode: StateNode): void {
  [stateNode].map((target) => {});
}

// EmbedPreview.tsx
import { useMachine } from '@xstate/react';
import { createModel } from 'xstate/lib/model';
import { send } from 'xstate';

const embedPreviewModel = createModel(
  {
    embedUrl: '',
  },
  {
    events: {
      PARAMS_CHANGED: (params: { foo: string }) => ({ params }),
      PREVIEW: () => ({}),
      IFRAME_LOADED: () => ({}),
      IFRAME_ERROR: () => ({}),
    },
  },
);

const embedPreviewMachine = embedPreviewModel.createMachine({
  entry: send('ada'),
});

useMachine(embedPreviewMachine, {
  actions: {
    saveParams: embedPreviewModel.assign({
      embedUrl: (_, e) => (e as any).params,
    }),
  },
});

πŸ™ Actual behavior

When running yarn tsc I get an error at src/EmbedPreview.tsx:23:12, but if I make any of those changes the error "goes away":

  • remove App.tsx
  • rename App.tsx to something coming after EmbedPreview.tsx (alphabetically)
  • remove the unused target parameter in the callback within toDirectedGraph in the App.tsx

πŸ™‚ Expected behavior

Error reports should be consistent and not prone to the order in which files are loaded and in which some types are utilized (I suspect the latter is somehow related to some type info being lazily loaded/evaluated and that's why removing the callback parameter "helps").

More details

We've noticed a weird thing on this branch:
https://github.com/statelyai/xstate-viz/tree/ef97512b8d11b6bba752a287bf2efc6453a9a02e

The tsc has started to report a problem within this call:
https://github.com/statelyai/xstate-viz/blob/ef97512b8d11b6bba752a287bf2efc6453a9a02e/src/EmbedPreview.tsx#L147
However, the error could not be observed in the VScode.

I've made sure that VScode and tsc are running using the same TS version but yet still the VScode didn't report this issue. I've observed some more weird behaviors - after changing some lines of code I've seen some inferred values (related to this useMachine call) to change despite no code changes (start with code A, make changes, roll back -> different inference results at the beginning and at the end of the process).

So I've started to remove stuff from our project to get this somewhat minimal repro case. Somewhere during the process, VScode has started to show the error but I've noticed that now I seemingly irrelevant changes changed the output of the tsc and I've focused on that.

@sandersn sandersn added the Needs Investigation This issue needs a team member to investigate its status. label Oct 4, 2021
@andrewbranch
Copy link
Member

Did you see #46392 go through? I haven’t looked closely at this repro, but maybe try tomorrow with the latest nightly.

@Andarist
Copy link
Contributor Author

@andrewbranch thank you for looking into this. Unfortunately, this didn't help here :p The updated repro case with the latest nightly might be found here: statelyai/xstate-viz@f5ef6c3

I will try to minimize node_modules as well (as right now only the app code has been minimized) but this was already super time-consuming (which I understand is kinda irrelevant to you πŸ˜‰ ) and I'm unsure when I will be able to play with this further.

This is currently not a blocker for me, only something very strange that I've observed, and that, IMHO, is a clear sign of a bug somewhere. As a user, I would expect those kinds of things to be deterministic.

Logs from updating TS, depicting exact steps that I've done to verify the broken behavior
> yarn add --dev typescript@next

yarn add v1.22.10
[1/4] πŸ”  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] πŸ”—  Linking dependencies...
[4/4] πŸ”¨  Building fresh packages...
success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ typescript@4.5.0-dev.20211020
info All dependencies
└─ typescript@4.5.0-dev.20211020
$ patch-package && mkdir -p public/monaco-editor && cp -r node_modules/monaco-editor public
patch-package 6.4.7
Applying patches...
@supabase/gotrue-js@1.16.5 βœ”
@xstate/react@1.5.1 βœ”
xstate@4.23.0 βœ”
✨  Done in 6.28s.

> yarn tsc

yarn run v1.22.10
$ /xstate-viz-copy/node_modules/.bin/tsc
src/EmbedPreview.tsx:23:12 - error TS2345: Argument of type 'StateMachine<{ embedUrl: string; }, any, EventFromEventCreators<FinalEventCreators<{ PARAMS_CHANGED: (params: { foo: string; }) => { params: { foo: string; }; }; PREVIEW: () => {}; IFRAME_LOADED: () => {}; IFRAME_ERROR: () => {}; }>>, any>' is not assignable to parameter of type 'StateMachine<any, any, never, any>'.
  Types of property 'withConfig' are incompatible.
    Type '(options: Partial<MachineOptions<{ embedUrl: string; }, EventFromEventCreators<FinalEventCreators<{ PARAMS_CHANGED: (params: { foo: string; }) => { params: { foo: string; }; }; PREVIEW: () => {}; IFRAME_LOADED: () => {}; IFRAME_ERROR: () => {}; }>>>>, context?: { ...; } | undefined) => StateMachine<...>' is not assignable to type '(options: Partial<MachineOptions<any, never>>, context?: any) => StateMachine<any, any, never, any>'.
      Types of parameters 'options' and 'options' are incompatible.
        Type 'Partial<MachineOptions<any, never>>' is not assignable to type 'Partial<MachineOptions<{ embedUrl: string; }, EventFromEventCreators<FinalEventCreators<{ PARAMS_CHANGED: (params: { foo: string; }) => { params: { foo: string; }; }; PREVIEW: () => {}; IFRAME_LOADED: () => {}; IFRAME_ERROR: () => {}; }>>>>'.
          Types of property 'guards' are incompatible.
            Type 'Record<string, ConditionPredicate<any, never>> | undefined' is not assignable to type 'Record<string, ConditionPredicate<{ embedUrl: string; }, EventFromEventCreators<FinalEventCreators<{ PARAMS_CHANGED: (params: { foo: string; }) => { params: { foo: string; }; }; PREVIEW: () => {}; IFRAME_LOADED: () => {}; IFRAME_ERROR: () => {}; }>>>> | undefined'.
              Type 'Record<string, ConditionPredicate<any, never>>' is not assignable to type 'Record<string, ConditionPredicate<{ embedUrl: string; }, EventFromEventCreators<FinalEventCreators<{ PARAMS_CHANGED: (params: { foo: string; }) => { params: { foo: string; }; }; PREVIEW: () => {}; IFRAME_LOADED: () => {}; IFRAME_ERROR: () => {}; }>>>>'.

23 useMachine(embedPreviewMachine, {
              ~~~~~~~~~~~~~~~~~~~


Found 1 error.

error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

> mv src/App.tsx src/FApp.tsx 

> yarn tsc

yarn run v1.22.10
$ /xstate-viz-copy/node_modules/.bin/tsc
✨  Done in 2.76s.

@Andarist
Copy link
Contributor Author

This was likely a duplicate of #44572 and this also gets fixed by #45628 (I've verified that).

@Andarist
Copy link
Contributor Author

Andarist commented Mar 8, 2022

I've verified that this now gets fixed by #48080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants