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

[Table] Should we make TableContext public? #24852

Closed
2 tasks done
jschlieber-innio opened this issue Feb 10, 2021 · 4 comments
Closed
2 tasks done

[Table] Should we make TableContext public? #24852

jschlieber-innio opened this issue Feb 10, 2021 · 4 comments
Labels
component: table This is the name of the generic UI component, not the React module! waiting for 👍 Waiting for upvotes

Comments

@jschlieber-innio
Copy link

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When importing import TableContext from "@material-ui/core/Table/TableContext"; and then doing useContext(TableContext) within a MaterialUI Table component, e.g.:

import TableContext from "@material-ui/core/Table/TableContext";
import TableCell from "@material-ui/core/TableCell";
import Checkbox from "@material-ui/core/TableCell";

// this will be used somewhat deeply nested inside of a Table component
function MyTableCell() {
  const table = useContext(TableContext);
  // will always be undefined
  console.log("table context", table);
  const size = table?.size ?? 'medium';
  return (
    <TableCell>
      <Checkbox size={size} />
    </TableCell>;
}

it seems that the imported TableContext is not equal to the one used to render it's corresponding Provider in the Table component.

I also opened a StackOverflow question related to this because I wanted to see if I missed something obvious: https://stackoverflow.com/questions/66078662/usecontexttablecontext-returns-undefined

Expected Behavior 🤔

From reading the MaterialUI source I would conclude it should be possible to use the TableContext as described in the above example. So this might be some issue with the distribution leading to having multiple non equal TableContexts...

Steps to Reproduce 🕹

https://codesandbox.io/s/quiet-tdd-g2kb4?file=/demo.tsx

Steps:

  1. run the code sandbox
  2. check the console (table context will always give undefined)

Context 🔦

I would like to be able to access the TableContext from a deeply nested tree to better align table content to the table size. I'm aware that I could use a custom context to achieve the same, but it just seems kind of odd to not be able to use the already existing TableContext.

Environment 🌎

`npx @material-ui/envinfo`
System:
    OS: macOS 11.2
  Binaries:
    Node: 14.15.4 - ~/.nvm/versions/node/v14.15.4/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.10 - ~/.nvm/versions/node/v14.15.4/bin/npm
  Browsers:
    Chrome: 88.0.4324.150
    Edge: Not Found
    Firefox: 80.0
    Safari: 14.0.3
  npmPackages:
    @emotion/react: ^11.1.4 => 11.1.5
    @emotion/styled: ^11.0.0 => 11.1.5
    @material-ui/core: ^4.11.0 => 4.11.3
    @material-ui/icons: ^4.9.1 => 4.11.2
    @material-ui/styles:  4.11.3
    @material-ui/system:  4.11.3
    @material-ui/types:  5.1.0
    @material-ui/utils:  4.11.2
    @types/react:  17.0.1
    react: ^17.0.1 => 17.0.1
    react-dom: ^17.0.1 => 17.0.1
    typescript: ^4.1.3 => 4.1.3
@jschlieber-innio jschlieber-innio added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 10, 2021
@jschlieber-innio jschlieber-innio changed the title useContext(TableContext) returns undefined [Table] useContext(TableContext) returns undefined Feb 10, 2021
@oliviertassinari oliviertassinari added the component: table This is the name of the generic UI component, not the React module! label Feb 10, 2021
@oliviertassinari oliviertassinari changed the title [Table] useContext(TableContext) returns undefined [Table] Should we make TableContext public? Feb 10, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 10, 2021

When importing import TableContext from "@material-ui/core/Table/TableContext";

@jschlieber-innio Imports that are more than one level deep are private. TableContext is private. In your case, it likely doesn't work because you bundle two different versions CJS and ESM by doing so (another reason to no import nested => bloat).

This issue sounds a lot like #23888 that is about accessing AccordionContext or #19861 about accessing FormControlContext (made public by @eps1lon in #16503 with a hook).

@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 10, 2021
@oliviertassinari
Copy link
Member

I would be tempted to close with a "waiting for upvotes" label. It seems to be an advanced usage.

@eps1lon
Copy link
Member

eps1lon commented Feb 11, 2021

I'm aware that I could use a custom context to achieve the same, but it just seems kind of odd to not be able to use the already existing TableContext.

I would recommend doing this until you find limitations. Then we can discuss how to best solve these limitations. Making something public locks the implementation which increases maintenance burden which affects every user.

@oliviertassinari oliviertassinari added the waiting for 👍 Waiting for upvotes label Feb 11, 2021
@simenandresen
Copy link

I would really love if TableContext was made public. I have a couple of use cases for it already, were TableContext seems like an obvious solution (mainly just wanted to get the current size so I could use the same size in a nested table)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: table This is the name of the generic UI component, not the React module! waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

4 participants