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

[test] Update matchMedia mocks #43240

Merged
merged 6 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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
4 changes: 2 additions & 2 deletions packages-internal/test-utils/src/describeConformance.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1034,8 +1034,8 @@ function describeConformance(
});
window.matchMedia = () =>
({
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
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
4 changes: 2 additions & 2 deletions packages/mui-joy/src/styles/CssVarsProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ describe('[Joy] CssVarsProvider', () => {
});
window.matchMedia = () =>
({
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
}) as unknown as MediaQueryList;
});

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-joy/src/styles/extendTheme.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ describe('extendTheme', () => {
configurable: true,
});
window.matchMedia = () => ({
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-material/src/styles/ThemeProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ describe('ThemeProvider', () => {
});
window.matchMedia = () =>
({
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
}) as unknown as MediaQueryList;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ describe('[Material UI] ThemeProviderWithVars', () => {
configurable: true,
});
window.matchMedia = () => ({
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-material/src/styles/extendTheme.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ describe('extendTheme', () => {
configurable: true,
});
window.matchMedia = () => ({
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});
});

Expand Down
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
4 changes: 2 additions & 2 deletions packages/mui-system/src/cssVars/createCssVarsProvider.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ describe('createCssVarsProvider', () => {
let storage = {};
const createMatchMedia = (matches) => () => ({
matches,
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});

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

const createMatchMedia = (matches) => () => ({
matches,
addListener: (listener) => {
addEventListener: (listener) => {
trigger = listener;
},
removeListener: () => {},
removeEventListener: () => {},
});

before(() => {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/mui-system/theme-scoping.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe('Multiple nested theme providers', () => {
let storage: Record<string, string> = {};
const createMatchMedia = (matches: boolean) => () => ({
matches,
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
});

beforeEach(() => {
Expand Down
Loading