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

[Feature]: Allow partial selectors for react-context-selector #32132

Closed
1 task done
seanpmaxwell opened this issue Jul 28, 2024 · 1 comment
Closed
1 task done

[Feature]: Allow partial selectors for react-context-selector #32132

seanpmaxwell opened this issue Jul 28, 2024 · 1 comment

Comments

@seanpmaxwell
Copy link

Library

React Components / v9 (@fluentui/react-components)

Describe the feature that you would like added

Coming from a redux environment, I was used to doing a lot of this:

  const {
    session,
    signupMethodStr,
  } = useAppCtx(ctx => ({
    session: ctx.session,
    signupMethodStr: ctx.session.signupMethodStr,
  }));

Which is a lot nicer/cleaner that having to call the useAppCtx() hook multiple times. This causes a too many re-renders error in ReactJS? error though. I believe it's because new object gets created every time the selector is run. In order to fix this I had to do something hacky with the selector function:

export const useAppCtx = <T extends {}>(selector: TSlctr<T>): T => {
  const ctx = useContextSelector(AppCxt, ctx => ctx);
  return selector(ctx);
};

Instead of how the docs show:

export const useAppCtx = <T extends {}>(selector: TSlctr<T>): T => {
  return useContextSelector(AppCxt, selector);
};

Is there any chance we get could this to work by default? Thanks.

Have you discussed this feature with our team

No

Additional context

No response

Validations

  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Priority

None

@layershifter
Copy link
Member

layershifter commented Jul 29, 2024

@seanpmaxwell it's something that have been implemented in Fluent UI v0 (see https://github.com/microsoft/fluentui/blob/ceacd96379e6d7268d1807a318a9b6f7c48f519b/packages/fluentui/react-bindings/src/context-selector/useContextSelectors.ts), but we found out that the pattern is fragile and is hard to use properly.

I hope that it clarifies why we are not going to implement it 🐱


FYI:

  const {
    session,
    signupMethodStr,
  } = useAppCtx(ctx => ({
    session: ctx.session,
    signupMethodStr: ctx.session.signupMethodStr,
  }));

Which is a lot nicer/cleaner that having to call the useAppCtx() hook multiple times. This causes a too many re-renders error in ReactJS? error though.

This is broken and will cause a component to render on an every re-render as a result of a selector function is never referentially equal.

@layershifter layershifter closed this as not planned Won't fix, can't repro, duplicate, stale Jul 29, 2024
@layershifter layershifter added the Status: Not on Roadmap Issue is not currently on roadmap, no fix planned label Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants