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

Need better linting for "Snapshots in callbacks are not recommended" when importing another file #39

Open
devnomic opened this issue Jan 6, 2023 · 4 comments

Comments

@devnomic
Copy link

devnomic commented Jan 6, 2023

Lets say i have a hook file with a state in it. useAppState.ts

export const state = proxy({
  isLocked: false,
});

export const useAppState = () => {
  const snap = useSnapshot(state);
  return {
    ...snap,
    state,
  };
};

File 2 SomeComponent.tsx

const { state, isLocked } = useAppState();

const handleClick = () => {
   console.log(isLocked); // No warning
};

Hopefully this can be resolved also, as this will probably lead to hard to catch bugs.

@dai-shi
Copy link
Member

dai-shi commented Jan 6, 2023

Hi, thanks for opening this up.

  return {
    ...snap,
    state,
  };

That seems like almost impossible to track.

const { state, isLocked } = useAppState();

const handleClick = () => {
   console.log(isLocked); // No warning
};

It looks actually fine if isLocked isn't an object.

Need better linting for "Snapshots in callbacks are not recommended" when importing another file

Oh, do you mean, it's a false positive?

@devnomic
Copy link
Author

devnomic commented Jan 6, 2023

Hi @dai-shi

Thanks for replying, no it was not false positive.
It was detected just fine by eslint, if the state is not imported.
don't know why?

e.g.

const state = proxy({
  isLocked: false,
});

function SomeComponent() {
   const { isLocked } = useSnapshot(state);
   const handleClick = () => {
     console.log(isLocked); // Correct, lint warn
  };
}

@devnomic
Copy link
Author

devnomic commented Jan 6, 2023

I think maybe not detected because of spreading in other object?

@dai-shi
Copy link
Member

dai-shi commented Jan 6, 2023

I see. (I misunderstood something in my previous comment.)

Yeah, maybe. Can you try without spreading?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants