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.

Sign up for GitHub

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

Jump to bottom

[test] Update matchMedia mocks #43240

Merged
merged 6 commits into from
Aug 13, 2024
Merged

[test] Update matchMedia mocks #43240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f9e591
stop using deprecated events methods
cherniavskii Aug 9, 2024
d29041c
Merge branch 'next' into describe-conformance-media-query
cherniavskii Aug 9, 2024
f13e2ce
mock deprecated methods in describeConformance as well
cherniavskii Aug 9, 2024
225876f
mock deprecated methods as well
cherniavskii Aug 9, 2024
820167d
Merge branch 'next' into describe-conformance-media-query
cherniavskii Aug 13, 2024
ed26c3e
add comments
cherniavskii Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/data/material/components/use-media-query/use-media-query.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ function createMatchMedia(width) {
matches: mediaQuery.match(query, {
width,
}),
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});
}

Expand Down
3 changes: 3 additions & 0 deletions packages-internal/test-utils/src/describeConformance.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,8 +1034,11 @@ function describeConformance(
});
window.matchMedia = () =>
({
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, it should mock both here to work with both v5 and v6.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow. This PR is supposed to land on next (v6). How does keeping addListener/removeListener help v5?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the current release strategy for the internal packages, this should probably be a major release of the @mui/internal-test-utils and dropping the addListener altogether as the linked PR for removing Safari support did. 🤔
What do others think?
cc @mui/code-infra

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is supposed to land on next (v6). How does keeping addListener/removeListener help v5?

We're using the @mui/internal-test-utils released from the next branch, but we can run our tests using either @mui/material v5 or v6. Since @mui/material@5 still uses addListener/removeListener, we need both to run the tests against both v5 and v6.
Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the current release strategy for the internal packages, this should probably be a major release of the @mui/internal-test-utils

We're not following semver with internal packages. The strategy is to be always backwards compatible (*). So I believe supporting both as @cherniavskii does makes most sense to me 👍

(*) backwards compatible against our own code that is. As soon as none of our code uses the compatibility mode anymore, it can be dropped, even on a minor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good 👍🏻
This PR is ready for review then.

}) as unknown as MediaQueryList;
});
afterEach(() => {
Expand Down
3 changes: 3 additions & 0 deletions packages/mui-joy/src/styles/CssVarsProvider.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ describe('[Joy] CssVarsProvider', () => {
});
window.matchMedia = () =>
({
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
}) as unknown as MediaQueryList;
});

Expand Down
3 changes: 3 additions & 0 deletions packages/mui-joy/src/styles/extendTheme.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,11 @@ describe('extendTheme', () => {
configurable: true,
});
window.matchMedia = () => ({
// Keep mocking legacy methods because older code might still use them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
});
});

Expand Down
3 changes: 3 additions & 0 deletions packages/mui-material/src/styles/ThemeProvider.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ describe('ThemeProvider', () => {
});
window.matchMedia = () =>
({
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
}) as unknown as MediaQueryList;
});

Expand Down
3 changes: 3 additions & 0 deletions packages/mui-material/src/styles/ThemeProviderWithVars.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ describe('[Material UI] ThemeProviderWithVars', () => {
configurable: true,
});
window.matchMedia = () => ({
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
});
});

Expand Down
3 changes: 3 additions & 0 deletions packages/mui-material/src/styles/extendTheme.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ describe('extendTheme', () => {
configurable: true,
});
window.matchMedia = () => ({
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-system/src/InitColorSchemeScript/InitColorSchemeScript.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ describe('InitColorSchemeScript', () => {
let storage = {};
const createMatchMedia = (matches) => () => ({
matches,
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});

beforeEach(() => {
Expand Down
3 changes: 3 additions & 0 deletions packages/mui-system/src/cssVars/createCssVarsProvider.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ describe('createCssVarsProvider', () => {
let storage = {};
const createMatchMedia = (matches) => () => ({
matches,
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
});

beforeEach(() => {
Expand Down
5 changes: 5 additions & 0 deletions packages/mui-system/src/cssVars/useCurrentColorScheme.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@ describe('useCurrentColorScheme', () => {

const createMatchMedia = (matches) => () => ({
matches,
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: (listener) => {
trigger = listener;
},
addEventListener: (listener) => {
trigger = listener;
},
removeListener: () => {},
removeEventListener: () => {},
});

before(() => {
Expand Down
3 changes: 3 additions & 0 deletions test/integration/mui-system/theme-scoping.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,11 @@ describe('Multiple nested theme providers', () => {
let storage: Record<string, string> = {};
const createMatchMedia = (matches: boolean) => () => ({
matches,
// Keep mocking legacy methods because @mui/material v5 still uses them
addListener: () => {},
addEventListener: () => {},
removeListener: () => {},
removeEventListener: () => {},
});

beforeEach(() => {
Expand Down