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

Bug in @preact/signals v2.0.1 and v1.3.2 breaking material-react-table and @material-table/core functionality #650

Open
2 of 5 tasks
pinchasr2022 opened this issue Jan 28, 2025 · 5 comments

Comments

@pinchasr2022
Copy link

pinchasr2022 commented Jan 28, 2025

  • Check if updating to the latest version resolves the issue
    current: 2.0.1

Environment

  • I am using @preact/signals-core
  • I am using @preact/signals
  • I am using @preact/signals-react

Describe the bug
There is a bug introduced in versions @preact/signals 2.0.1 and 1.3.2, where certain table functionalities in popular libraries like material-react-table and @material-table/core stop working correctly. Specifically:

  • In material-react-table (v3.1.0), the search bar doesn't open when clicked, and core features like sorting, filtering, and grouping fail to function.
  • In @material-table/core (v6.4.4), the detailPanel feature, which expands table rows to show additional details, also breaks.

The bug is triggered when importing the functions computed or signal from @preact/signals, even if they are not actively used in the code. For example, the following import statement causes the issue:

import { computed, signal } from "@preact/signals";

To Reproduce
Please refer to my GitHub repository for a project demonstrating the issue:
GitHub Repository for Reproducing the Bug
Although the issue affects both libraries, my GitHub repository demonstrates the issue specifically with material-react-table.

Issue with material-react-table (Multiple Issues):

Steps to reproduce the behavior:

  1. Import computed or signal from @preact/signals:
import { computed, signal } from "@preact/signals";
  1. Implement the MaterialReactTable component in your project.
import { MaterialReactTable } from "material-react-table";

const Table = () => (
  <MaterialReactTable
    columns={[
      {
        accessorKey: "firstName",
        header: "First Name",
      },
      {
        accessorKey: "lastName",
        header: "Last Name",
      },
    ]}
    data={[
      {
        firstName: "John",
        lastName: "Doe",
      },
      {
        firstName: "Kevin",
        lastName: "Vandy",
      },
    ]}
  />
);

export default Table;
  1. Try using the search bar or the built-in features like filtering, sorting, grouping, etc.
  2. Issue: Clicking on the search bar does not open it, and other built-in features like filtering, sorting, and grouping do not work properly.

Issue with @material-table/core (Single Issue):

Steps to reproduce the behavior:

  1. Import computed or signal from @preact/signals:
import { computed, signal } from "@preact/signals";
  1. Implement the MaterialTable component in your project.
import MaterialTable from "@material-table/core";

const Table = () => (
  <MaterialTable
    columns={[
      {
        field: "firstName",
        title: "First Name",
      },
      {
        field: "lastName",
        title: "Last Name",
      },
    ]}
    data={[
      {
        firstName: "John",
        lastName: "Doe",
        email: "john@doe.com",
      },
      {
        firstName: "Kevin",
        lastName: "Vandy",
        email: "kevin@vandy.com",
      },
    ]}
    detailPanel={({ rowData }) => <div>{rowData.email}</div>}
  />
);

export default Table;
  1. Try using the detailPanel feature.
  2. Issue: The detailPanel feature does not expand the row to show additional details.
    Expected behavior
    When importing @preact/signals, table functionalities like search bars, sorting, filtering, and detailPanel should continue to work as expected, without any interruptions or malfunctions.
@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 28, 2025

What preact version are you using? The issue stems from #630 however when you are using hooks in any Preact older than 10.12.0 this issue should not be present. I can't currently really dig through that codebase as it's a bit unwieldly in the amount of dependencies it uses to achieve a table 😅 that being said, when hooks settle on the same value we bail https://github.com/preactjs/preact/blob/main/hooks/src/index.js#L252-L253, when there are no hooks we'll defer to the previous implementation of shouldComponentUpdate which is the signals one, the signals one will update for components that aren't using state and for components that are using signals and have pending updates.

Tried your repository in https://stackblitz.com/edit/vitejs-vite-ugzcpmhw?file=src%2Fapp.tsx,src%2FTable.tsx,src%2Fmain.tsx,src%2Findex.css&terminal=dev and it works perfectly

@pinchasr2022
Copy link
Author

pinchasr2022 commented Jan 29, 2025

Hey,

When I first encountered this issue, I was using Preact 10.12.0, but the problem persisted even after upgrading to 10.25.4.

One thing I noticed is that your setup on StackBlitz is using Vite as the bundler, whereas my project is built with Webpack.
It’s possible that the difference in bundlers is affecting how the hooks/signals behave in this case.

Would be great to know if there are any known differences in how Webpack vs. Vite handles this scenario.
Let me know if you need more details from my side!

Thanks for your time.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 29, 2025

The bundler should really not affect module behavior that would be a serious infraction on their responsibilities. I'd encourage you to check whether you really are on the latest Preact version

@pinchasr2022
Copy link
Author

I have confirmed that I am using Preact 10.25.4, but the issue still occurs.
Based on your explanation, the change in 10.12.0 (from #630) introduced this behavior.
Should it have been fixed in a later version? If so, which version specifically?

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jan 29, 2025

That's true, I would assume your webpack config has some odd configuration that mangles some property names or other. Normally however a bundler would not touch lib code, that being said, I can't really help you with that 😅

Even entertaining the base assumption here that webpack would mangle cross-library properties makes me feel like signals in general would not work. This all feels a bit like a long shot, I would see what these babel-plugins you are leveraging are doing.

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