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

ra-core causes @testing-library/react to be bundled in prod #4618

Closed
bard opened this issue Apr 1, 2020 · 5 comments
Closed

ra-core causes @testing-library/react to be bundled in prod #4618

bard opened this issue Apr 1, 2020 · 5 comments

Comments

@bard
Copy link
Contributor

bard commented Apr 1, 2020

What you were expecting:

Production bundle of an app containing react-admin to not contain @testing-library/react and its dependencies.

What happened instead:

@testing-library/react was included in a production bundle. As a side-effect, IE11 broke due to pretty-format (a dependency of @testing-library/react) containing code that relies on Symbol, which is not available in IE11.

Steps to reproduce:

Unfortunately none at the moment as I cannot share the project and this doesn't happen in a create-react-app project (which perhaps has a more sophisticated code elimination). Might be able to create a minimal test case later.

Related code:

I fixed this build by locally patching util/index.js at this point to not import and re-export renderWithRedux (which in turn depends on @testing-library/react).

Files needing renderWithRedux seem to be needed during dev only:

~/.../node_modules/ra-core$ ag 'import { renderWithRedux }'
src/i18n/useSetLocale.spec.js
9:import { renderWithRedux } from '../util';

src/i18n/useTranslate.spec.tsx
8:import { renderWithRedux } from '../util';

src/form/useChoices.spec.tsx
5:import { renderWithRedux } from '../util';

src/form/ValidationError.spec.tsx
8:import { renderWithRedux } from '../util';

src/controller/input/ReferenceArrayInputController.spec.tsx
5:import { renderWithRedux } from '../../util';

They could be fixed by having them import from ../util/renderWithRedux directly. Happy to provide a PR if that sounds sensible.

Also, @testing-library/react is reported in ra-core's package.json dependencies instead of devDependencies.

@fzaninotto
Copy link
Member

Thanks fro the report, see #3851 for the history of that dependency and the justification for bundling it to production.

The problem is that renderWithRedux is also used by ra-ui-material-ui. So not exporting it isn't a solution.

The IE11 bug is a real concern, though I suspect it can be fixed by a shim?

@bard
Copy link
Contributor Author

bard commented Apr 1, 2020

Hmm, I might be missing something but renderWithRedux in ra-ui-materialui seems to be only used in *.spec.* files, so only during dev, no?

.../ra-ui-materialui/src$ ag -l renderWithRedux
list/List.spec.js
list/FilterForm.spec.js
list/DatagridRow.spec.js
input/CheckboxGroupInput.spec.tsx
form/TabbedForm.spec.js
form/SimpleFormIterator.spec.js
form/SimpleForm.spec.js
form/FormTab.spec.js
field/SelectField.spec.js
field/ReferenceField.spec.js
detail/Show.spec.js
detail/Edit.spec.js
detail/Create.spec.js

@fzaninotto
Copy link
Member

right, but how do we make it available from ra-core to ra-ui-material-ui?

@bard
Copy link
Contributor Author

bard commented Apr 1, 2020

The smallest option would be:

modified   packages/ra-ui-materialui/src/input/CheckboxGroupInput.spec.tsx
@@ -3,267 +3,268 @@ import expect from 'expect';
 import CheckboxGroupInput from './CheckboxGroupInput';
 import { render, cleanup, fireEvent } from '@testing-library/react';
 import { Form } from 'react-final-form';
-import { renderWithRedux, TestTranslationProvider } from 'ra-core';
+import { TestTranslationProvider } from 'ra-core';
+import renderWithRedux from 'ra-core/src/util/renderWithRedux';

Though for consistency and clarity I guess I would do:

modified   packages/ra-ui-materialui/src/input/CheckboxGroupInput.spec.tsx
@@ -3,267 +3,267 @@ import expect from 'expect';
 import CheckboxGroupInput from './CheckboxGroupInput';
 import { render, cleanup, fireEvent } from '@testing-library/react';
 import { Form } from 'react-final-form';
-import { renderWithRedux, TestTranslationProvider } from 'ra-core';
+import { renderWithRedux, TestTranslationProvider } from 'ra-core/src/devUtils';

@fzaninotto
Copy link
Member

Fixed by #5846

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

3 participants