[TextField] Migrate FormHelperText to emotion

[duganbrett]

This PR migrates the FormHelperText component to the new emotion format as a part of #24405

• I have followed (at least) the PR section of the contributing guide.

[mui-pr-bot]

@material-ui/core: parsed: +0.17% , gzip: +0.19%

Details of bundle changes

Generated by 🚫 dangerJS against c05b91b

[duganbrett]

I'm probably missing something obvious but I'm not sure these tests are failing. It seems like it's working when testing the component manually.

[mnajdova]

I'm probably missing something obvious but I'm not sure these tests are failing. It seems like it's working when testing the component manually.

@duganbrett when you say it's working manually, do you mean you try to do syle overrides and theme variants?

[mnajdova]

I'm probably missing something obvious but I'm not sure these tests are failing. It seems like it's working when testing the component manually.

@duganbrett when you say it's working manually, do you mean you try to do syle overrides and theme variants?

I can ensure that the style overrides are working but the tests are failing. Here is a codesandbox - https://codesandbox.io/s/formpropstextfields-material-demo-forked-ke0sr?file=/demo.js

Here are the failing tests - https://app.circleci.com/jobs/github/mui-org/material-ui/221043?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

I tried skipping the tests in JSDOM, and at least locally the test:karma was passing successfully. I am not sure what is different about this component. I've pushed 935c276 to see if the CI will be green. cc @oliviertassinari @eps1lon any idea why this may be happening (it worked as expected for all components we migrated so far...)

[eps1lon]

@eps1lon any idea why this may be happening (it worked as expected for all components we migrated so far...)

test_browser was green on 935c276: https://app.circleci.com/pipelines/github/mui-org/material-ui/37358/workflows/1d716975-2d25-4189-ab36-89abcf9fb863/jobs/221058. What am I missing?

Generally, rather push the failing build than a green build with skips. With a failing build you have the failure to start with. With a green build you don't know where to start without looking through all changes.

[eps1lon]

A question while I'm here: Do these need to be interfaces for module augmentation or would a Record<FormHelperTextClassKey, string> be sufficient?

[mnajdova]

That's a good point, I believe we can use the same. It will also ensure that the typings on the classses are correct.__

[mnajdova]

I wonder how we should best handle this. So far the components were re-introducing this new typing. In order not to block the migration, should we handle this change in the end for all components once they are migrated? I will leave a comment on the issue for this change.

[oliviertassinari]

I think that we could try to migrate one component to this approach, and if it works well, migrate them all at once? I could do it this weekend if needed.

[mnajdova]

Sounds good to me 👍 will merge this one then and we have this #24405 (comment) for reference of what needs to be done

[mnajdova]

@eps1lon I've linked the failing build - https://app.circleci.com/jobs/github/mui-org/material-ui/221043?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link thought it would be easier if I can link the failing and the commit that fixed it (skipped JSDOM).

I've reverted the changes with 7b8124c the build should be identical with the one linked above

[eps1lon]

(skipped JSDOM).

But you've said test:karma was at leat passing locally so I suspected it's a problem with test_browser not test_unit.

Failing build that's blocking is simpler. I just can inspect the last workflow instead of reading through comments.

[eps1lon]

I am not sure what is different about this component.

I suspect that it has to do with how styles cascade. That's probably not that clear from "since JSDOM does not implement the Cascade" in the error message. "Cascade" is jargon in CSS and describes how multiple style declarations for the same element interact with each other. Would love to link to https://developer.mozilla.org/en-US/docs/Web/CSS/Cascade in the error message. In a perfect world I would link the a more in-depth markdown text but I don't see how this results in more people trying to understand the problem.

[mnajdova]

I suspect that it has to do with how styles cascade. That's probably not that clear from "since JSDOM does not implement the Cascade" in the error message. "Cascade" is jargon in CSS and describes how multiple style declarations for the same element interact with each other. Would love to link to https://developer.mozilla.org/en-US/docs/Web/CSS/Cascade in the error message. In a perfect world I would link the a more in-depth markdown text but I don't see how this results in more people trying to understand the problem.

In will then prepare a PR where these tests (the conformance) will skip JSDOM.

[eps1lon]

In will then prepare a PR where these tests (the conformance) will skip JSDOM.

Yeah, I think running style related tests in JSDOM is futile. I don't think this will ever get better. But we might get a faster iteration speed for browser test to the point where it's faster to run test:karma locally. For now skipping JSDOM is fine.