[typescript] Dependency on enzyme should be abstracted

[shousper]

Problem description

The typings contain a dependency on enzyme that would require anyone using material-ui with typescript to also install @types/enzyme, and also enzyme for testing.

Steps to reproduce

N/A

Versions

• Material-UI: 1.0.0-beta.6
• React: N/A
• Browser: N/A

Description

I would like to suggest the testing utils specific to enzyme be moved into another package (e.g. material-ui-enzyme). Any testing utilities that come with material-ui should probably only depend on React itself so as to leave developers the freedom of choice with it comes to testing.

Images & references

N/A

[sebald]

I guess the question is how many people have installed enzyme anyway vs. who doesn't and then make changes, so the majority isn't bothered by this @oliviertassinari Previously, the types for enzyme and react were part to the dependencies of material-ui, but, since most people don't use TS, they were moved to the devDependencies.

Because the test-utils are part of material-ui's "core", it would be much weirder to have them as a separate package. Especially, because they're only types and do not start with the usual @types/....
But, maybe it's a good idea to have them as a standalone package anyway? :)

Also, you don't need to install enzyme only the typings.

[oliviertassinari]

Honestly, I'm no longer sure that we should be keeping exposing the test-utils/ folder. As we have removed the requirement for the theme to be present in the context. I think that we would be better of hiding the test helpers. This way, it will be easier as maintainers to work on the test suite.

[oliviertassinari]

Also, you don't need to install enzyme only the typings.

👍

[sebald]

Sounds good. Will not including the test-utils folder into the build suffice?

[oliviertassinari]

@sebald I guess. But most of the work is going to be updating the docs.

[shousper]

Also, you don't need to install enzyme only the typings.

I do if I want to use your test-utils :(

[oliviertassinari]

Oh, so you do think that this testing helpers functions can be beneficial to our users?
We don't want to hide the enzyme dependency. I do want people to manualy install enzyme when using our tests helpers. Still, with another package (e.g. material-ui-test) , we can control the peer dependency, this is better.

[sebald]

@oliviertassinari Me? No, I do really like them. The test-utils typings were one of the first I wrote! Before refactoring the styling stuff, they were necessary for testing, I think. Now we mostly use enzyme directly. But getClasses still is super useful!

@shousper why do you have issues with the additional dependency of enzyme if you're using the test-utils anyway?

[shousper]

@sebald Sorry, I meant that in the non-direct sense. i.e. if I were someone wanting to use some test utilities specifically for material-ui, I'd have to also use enzyme. But I don't mean me specifically :)

[oliviertassinari]

Alright, I'm going to close the issue. To sum up:

• We want to enzyme dependency of the test helpers to be explicit
• Some might find the test helpers useful, so we keep them
• It would be better in terms of package delivery to ship the tests helpers into their own material-ui-test package (so we can have a finer grain control on the dependency and peer dependency). This is something we started discussing here Build as a multi-repository #7839. But, this has an important maintenance cost. For now, I do think that it would be better to wait and see if that worth it.

Thanks for the discussion, we can always reopen the issue if you don't agree, or want to push into a different direction.