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

[DataGrid] Add Preact support #1881

Closed
1 task done
oliviertassinari opened this issue Jun 12, 2021 · 3 comments
Closed
1 task done

[DataGrid] Add Preact support #1881

oliviertassinari opened this issue Jun 12, 2021 · 3 comments
Labels
new feature New feature or request waiting for 👍 Waiting for upvotes

Comments

@oliviertassinari
Copy link
Member

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Support Preact.

Examples 🌈

The core components already aim/support Preact.

Motivation 🔦

  • Bundle size matters. It also resonates with the idea that installing 200k gzipped for a simple data grid is completely KO.
  • It could even be important for us if we need to expose an Angular or Vue wrapper with Preact.

Context

Related issues: preactjs/preact#3055. Quick exploration #1828.

@oliviertassinari oliviertassinari added new feature New feature or request waiting for 👍 Waiting for upvotes low priority and removed waiting for 👍 Waiting for upvotes labels Jun 12, 2021
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jun 12, 2021

For anyone that wants to work on it. The issue can be reproduced in the docs with:

diff --git a/docs/next.config.js b/docs/next.config.js
index 3c83e0a6..6bfb03f1 100644
--- a/docs/next.config.js
+++ b/docs/next.config.js
@@ -4,6 +4,7 @@ const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer');
 const pkg = require('./node_modules/@material-ui/monorepo/package.json');
 const { findPages } = require('./src/modules/utils/find');
 const { LANGUAGES, LANGUAGES_SSR } = require('./src/modules/constants');
+const withPreact = require('next-plugin-preact');

 const workspaceRoot = path.join(__dirname, '../');

@@ -20,7 +21,7 @@ const reactMode = 'legacy';
 // eslint-disable-next-line no-console
 console.log(`Using React '${reactMode}' mode.`);

-module.exports = {
+module.exports = withPreact({
   typescript: {
     // Motivated by https://github.com/zeit/next.js/issues/7687
     ignoreDevErrors: true,
@@ -161,4 +162,4 @@ module.exports = {
       { source: '/api/:rest*', destination: '/api-docs/:rest*' },
     ];
   },
-};
+});
diff --git a/docs/package.json b/docs/package.json
index 082f1332..313e7cee 100644
--- a/docs/package.json
+++ b/docs/package.json
@@ -61,9 +61,9 @@
     "prismjs": "^1.17.1",
     "prop-types": "^15.7.2",
     "raw-loader": "^1.0.0",
-    "react": "^17.0.2",
+    "react": "npm:@preact/compat",
     "react-docgen": "^5.0.0-beta.1",
-    "react-dom": "^17.0.2",
+    "react-dom": "npm:@preact/compat",
     "react-is": "^17.0.2",
     "react-redux": "^7.1.1",
     "react-router": "^5.0.0",
diff --git a/docs/pages/index.js b/docs/pages/index.js
index 36a9a091..2be75d7f 100644
--- a/docs/pages/index.js
+++ b/docs/pages/index.js
@@ -1,3 +1,45 @@
-import LandingPage from '@material-ui/monorepo/docs/pages/index';
+import { render } from "preact";
+import { DataGrid } from "@material-ui/data-grid";

-export default LandingPage;
+const columns = [
+  { field: "id", headerName: "ID", width: 70 },
+  { field: "firstName", headerName: "First name", width: 130 },
+  { field: "lastName", headerName: "Last name", width: 130 },
+  {
+    field: "age",
+    headerName: "Age",
+    type: "number",
+    width: 90
+  },
+  {
+    field: "fullName",
+    headerName: "Full name",
+    description: "This column has a value getter and is not sortable.",
+    sortable: false,
+    width: 160,
+    valueGetter: (params) =>
+      `${params.getValue("firstName") || ""} ${
+        params.getValue("lastName") || ""
+      }`
+  }
+];
+
+const rows = [
+  { id: 1, lastName: "Snow", firstName: "Jon", age: 35 },
+  { id: 2, lastName: "Lannister", firstName: "Cersei", age: 42 },
+  { id: 3, lastName: "Lannister", firstName: "Jaime", age: 45 },
+  { id: 4, lastName: "Stark", firstName: "Arya", age: 16 },
+  { id: 5, lastName: "Targaryen", firstName: "Daenerys", age: null },
+  { id: 6, lastName: "Melisandre", firstName: null, age: 150 },
+  { id: 7, lastName: "Clifford", firstName: "Ferrara", age: 44 },
+  { id: 8, lastName: "Frances", firstName: "Rossini", age: 36 },
+  { id: 9, lastName: "Roxie", firstName: "Harvey", age: 65 }
+];
+
+export default function App() {
+  return (
+    <div style={{ height: 300, width: "100%" }}>
+      <DataGrid rows={rows} columns={columns} pageSize={5} checkboxSelection />
+    </div>
+  );
+}
diff --git a/package.json b/package.json
index 1244d8c8..c05e702e 100644
--- a/package.json
+++ b/package.json
@@ -115,8 +115,8 @@
     "playwright": "^1.6.1",
     "prettier": "^2.0.5",
     "pretty-format-v24": "npm:pretty-format@24",
-    "react": "^17.0.2",
-    "react-dom": "^17.0.2",
+    "react": "npm:@preact/compat",
+    "react-dom": "npm:@preact/compat",
     "rollup": "^2.28.2",
     "rollup-plugin-cleaner": "^1.0.0",
     "rollup-plugin-command": "^1.1.3",
@@ -150,5 +150,10 @@
     "nohoist": [
       "**/@material-ui/monorepo"
     ]
+  },
+  "dependencies": {
+    "next-plugin-preact": "^3.0.6",
+    "preact": "^10.5.13",
+    "preact-render-to-string": "^5.1.19"
   }
 }

And also to fix the inifinit loop when an error trigger, empty the error boundary:

diff --git a/packages/grid/_modules_/grid/components/ErrorBoundary.tsx b/packages/grid/_modules_/grid/components/ErrorBoundary.tsx
index 8f6cd9ed..97b2da7b 100644
--- a/packages/grid/_modules_/grid/components/ErrorBoundary.tsx
+++ b/packages/grid/_modules_/grid/components/ErrorBoundary.tsx
@@ -12,28 +12,6 @@ export interface ErrorBoundaryProps {
 }

 export class ErrorBoundary extends React.Component<ErrorBoundaryProps, any> {
-  static getDerivedStateFromError(error) {
-    // Update state so the next render will show the fallback UI.
-    return { hasError: true, error };
-  }
-
-  componentDidCatch(error: Error, errorInfo: ErrorInfo) {
-    if (this.props.api.current) {
-      this.logError(error);
-
-      // Allows to trigger the Error event and all listener can run on Error
-      this.props.api.current.showError({ error, errorInfo });
-    }
-  }
-
-  logError(error: Error, errorInfo?: ErrorInfo) {
-    this.props.logger.error(
-      `An unexpected error occurred. Error: ${error && error.message}. `,
-      error,
-      errorInfo,
-    );
-  }
-
   render() {
     if (this.props.hasError || this.state?.hasError) {
       // You can render any custom fallback UI

Otherwise, on codesandbox:

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jun 12, 2021

@dtassone After a quick exploration, what I could see is that

  1. The failing error is different since [DataGrid] Exploring Plugin api #1720

Capture d’écran 2021-06-12 à 13 50 38

  1. Preact doesn't respect the order of useLayoutEffect vs. useEffect between children and parent. I could patch it with:
diff --git a/packages/grid/_modules_/grid/components/containers/GridRoot.tsx b/packages/grid/_modules_/grid/components/containers/GridRoot.tsx
index e1a56370..fbb9ba62 100644
--- a/packages/grid/_modules_/grid/components/containers/GridRoot.tsx
+++ b/packages/grid/_modules_/grid/components/containers/GridRoot.tsx
@@ -30,7 +30,7 @@ export const GridRoot = React.forwardRef<HTMLDivElement, GridRootProps>(function
   apiRef.current.rootElementRef = rootContainerRef;

   return (
-    <NoSsr>
+    <NoSsr defer>
       <div
         ref={handleRef}
         className={clsx(classes.root, options.classes?.root, className, classNameProp, {

It seems to be very close to this bug preactjs/preact#3179.

  1. Then it fails here:

Capture d’écran 2021-06-12 à 13 52 06


I feel like Preact support for the data grid is a dead end, unless:

  1. Preact gets closer to how React behaves.
  2. We get stricter about how we write the internal of the data grid. For instance, this apiRef that we mutate is not React idiomatic, not at all (the first fails with Preact). But we did it anyway because we need to have great performance too.

@cherniavskii
Copy link
Member

I've just learned about this framework Fresh https://deno.com/blog/fresh-is-stable built which is the Preact framework supported by Deno and found this issue.
We might reconsider Preact support for MUI X if Fresh gets adopted by the community.
Here's a codesandbox with data grid v5 that doesn't work with Preact: https://codesandbox.io/s/bold-gauss-flp34f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

2 participants