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

Tabbed form error styling not working on certain production builds #7226

Closed
archfz opened this issue Feb 10, 2022 · 2 comments
Closed

Tabbed form error styling not working on certain production builds #7226

archfz opened this issue Feb 10, 2022 · 2 comments
Labels

Comments

@archfz
Copy link

archfz commented Feb 10, 2022

What you were expecting:
Tabbed form tab errors to work properly, even with bundler import optimizations.

What happened instead:
Tabbed form tab error styling is not working on production builds. Now I know that this issue was already submitted once here #5428 . But form this thread it seems that it's not a react admin issue. I think it is a react admin issue, or atleast one that can only be properly solved by react admin. Bundler optimizations are good and expected.

The real issue is that classes props are used and sent downstream to components that are cloned. There are two components in hand: TabbedForm and FormTab. TabbedForm is the import that defines the makesStyles with the error styling and in turn passes it down through FormTab into TabbedFormTabs into FormTabHeader. FormTabHeader is the one that imports Tab from material-ui, which in turn defines it's makeStyles having the default styling.

Now as you probably know the order in which makeStyles are executed defines also their specificity. Since in react admin there is no dependency graph between the TabbedForm and FormTab it is up to the client code to define in which order these are imported. Also since this dependency is usually loaded at the same module level, control of the order is hardly in the hands on the developer and more in the hands of bundler, that could for optimization reasons load the modules in different order inside the requesting modules, especially since both TabbedForm and FormTab are loaded from the same package index (import { FormTab, TabbedForm } from 'react-admin';). So this is why I recommend this being fixed by react admin:

  1. Best would be to split the makeStyles in the TabbedForm and move inside the components that are actually using these classes. Either way best practice is to define styles just before the component that uses it. Is there any reasons for the tab error styling to be on such high level?
  2. Create a clear dependency graph between TabbedForm -> FormTab in react admin. I am pretty sure this is not quite possible, especially not desired since we want to allow custom components in place of FormTab.

I recommend the first approach, and it to be applied not just for this case, but for all cases. Here is the patch I am currently using:

diff --git a/node_modules/ra-ui-materialui/esm/form/FormTabHeader.js b/node_modules/ra-ui-materialui/esm/form/FormTabHeader.js
index 59c47cb..2689030 100644
--- a/node_modules/ra-ui-materialui/esm/form/FormTabHeader.js
+++ b/node_modules/ra-ui-materialui/esm/form/FormTabHeader.js
@@ -1,3 +1,5 @@
+import {makeStyles} from "@material-ui/core/styles";
+
 var __assign = (this && this.__assign) || function () {
     __assign = Object.assign || function(t) {
         for (var s, i = 1, n = arguments.length; i < n; i++) {
@@ -27,7 +29,13 @@ import MuiTab from '@material-ui/core/Tab';
 import classnames from 'classnames';
 import { useTranslate, useFormGroup } from 'ra-core';
 import { useForm } from 'react-final-form';
+
+var useStyles = makeStyles(function (theme) { return ({
+    errorTabButton: { color: theme.palette.error.main },
+}); }, { name: 'RaFormTabHeader' });
+
 export var FormTabHeader = function (_a) {
+    var styles = useStyles();
     var _b;
     var classes = _a.classes, label = _a.label, value = _a.value, icon = _a.icon, className = _a.className, rest = __rest(_a, ["classes", "label", "value", "icon", "className"]);
     var translate = useTranslate();
@@ -44,7 +52,7 @@ export var FormTabHeader = function (_a) {
         return unsubscribe;
     }, [form, showError]);
     return (React.createElement(MuiTab, __assign({ label: translate(label, { _: label }), value: value, icon: icon, className: classnames('form-tab', className, (_b = {},
-            _b[classes.errorTabButton] = formGroup.invalid &&
+            _b[styles.errorTabButton] = formGroup.invalid &&
                 (formGroup.touched || showError) &&
                 location.pathname !== value,
             _b)), component: Link, to: __assign(__assign({}, location), { pathname: value }), id: "tabheader-" + value, "aria-controls": "tabpanel-" + value }, rest)));
diff --git a/node_modules/ra-ui-materialui/esm/form/TabbedForm.js b/node_modules/ra-ui-materialui/esm/form/TabbedForm.js
index 1d61bd9..19927ac 100644
--- a/node_modules/ra-ui-materialui/esm/form/TabbedForm.js
+++ b/node_modules/ra-ui-materialui/esm/form/TabbedForm.js
@@ -125,7 +125,6 @@ TabbedForm.propTypes = {
     sanitizeEmptyValues: PropTypes.bool,
 };
 var useStyles = makeStyles(function (theme) { return ({
-    errorTabButton: { color: theme.palette.error.main },
     content: {
         paddingTop: theme.spacing(1),
         paddingLeft: theme.spacing(2),

Steps to reproduce:
not sure unfortunately, I am using webpack with following optimizations:

  mode: 'production',
  optimization: {
    minimize: true,
    concatenateModules: true,
    splitChunks: {
      cacheGroups: {
        modules: {
          chunks: 'initial',
          name: 'modules',
          test: /node_modules/,
          enforce: true,
        },
      },
    },
  },

Environment

  • React-admin version: 3.19.7
  • Last version that did not exhibit the issue (if applicable):
  • React version: 16.8.18
  • Browser: firefox
  • Stack trace (in case of a JS error):
@fzaninotto
Copy link
Member

Thank you for the great bug report, analysis, and fix suggestion. We should definitely go for option 1.

@fzaninotto fzaninotto added the bug label Feb 11, 2022
@djhi djhi changed the title Tabbed form error styling not working on certain porduction builds Tabbed form error styling not working on certain production builds Feb 24, 2022
@fzaninotto fzaninotto added the v3 label Jul 22, 2022
@fzaninotto
Copy link
Member

With the release of react-admin v5, react-admin v3 has reached its end of life. We won't fix bugs or make any new release on the 3.x branch. We recommend that you switch to a more recent version of react-admin.

So I'm closing this issue as we won't fix it.

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

No branches or pull requests

2 participants