From 5fac22af7654f903edcb055e7cf74d909fe875fa Mon Sep 17 00:00:00 2001 From: Edo Rivai Date: Sun, 17 Mar 2019 11:30:10 +0100 Subject: [PATCH 1/2] First stab at backward compatibility of query prop --- modules/Media.js | 147 +++++++++---- ...ediaQueryList.js => MediaQueryListener.js} | 2 +- modules/__tests__/Media-test.js | 204 +++++++++++++++--- 3 files changed, 287 insertions(+), 66 deletions(-) rename modules/{MediaQueryList.js => MediaQueryListener.js} (94%) diff --git a/modules/Media.js b/modules/Media.js index ea727ba..abe9e2e 100644 --- a/modules/Media.js +++ b/modules/Media.js @@ -1,9 +1,9 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import invariant from 'invariant'; -import json2mq from 'json2mq'; +import React from "react"; +import PropTypes from "prop-types"; +import invariant from "invariant"; +import json2mq from "json2mq"; -import MediaQueryList from './MediaQueryList'; +import MediaQueryListener from "./MediaQueryListener"; const queryType = PropTypes.oneOfType([ PropTypes.string, @@ -16,8 +16,12 @@ const queryType = PropTypes.oneOfType([ */ class Media extends React.Component { static propTypes = { - defaultMatches: PropTypes.objectOf(PropTypes.bool), - queries: PropTypes.objectOf(queryType).isRequired, + defaultMatches: PropTypes.oneOfType([ + PropTypes.bool, + PropTypes.objectOf(PropTypes.bool) + ]), + query: queryType, + queries: PropTypes.objectOf(queryType), render: PropTypes.func, children: PropTypes.oneOfType([PropTypes.node, PropTypes.func]), targetWindow: PropTypes.object, @@ -29,22 +33,49 @@ class Media extends React.Component { constructor(props) { super(props); + invariant( + !(!props.query && !props.queries) || (props.query && props.queries), + ' must be supplied with either "query" or "queries"' + ); + + invariant( + props.defaultMatches === undefined || + !props.query || + typeof props.defaultMatches === "boolean", + " when query is set, defaultMatches must be a boolean, received " + + typeof props.defaultMatches + ); + + invariant( + props.defaultMatches === undefined || + !props.queries || + typeof props.defaultMatches === "object", + " when queries is set, defaultMatches must be a object of booleans, received " + + typeof props.defaultMatches + ); + if (typeof window !== "object") { - // In case we're rendering on the server + // In case we're rendering on the server, apply the default matches + let matches; + if (props.defaultMatches) { + matches = props.defaultMatches; + } else if (props.query) { + matches = true; + } /* if (props.queries) */ else { + matches = Object.keys(this.props.queries).reduce( + (acc, key) => ({ ...acc, [key]: true }), + {} + ); + } this.state = { - matches: - this.props.defaultMatches || - Object.keys(this.props.queries).reduce( - (acc, key) => ({ ...acc, [key]: true }), - {} - ) + matches }; return; } this.initialize(); - // Instead of calling this.updateMatches, we manually set the state to prevent + // Instead of calling this.updateMatches, we manually set the initial state to prevent // calling setState, which could trigger an unnecessary second render this.state = { matches: @@ -52,40 +83,51 @@ class Media extends React.Component { ? this.props.defaultMatches : this.getMatches() }; + this.onChange(); } getMatches = () => { - return this.queries.reduce( - (acc, { name, mqList }) => ({ ...acc, [name]: mqList.matches }), + const result = this.queries.reduce( + (acc, { name, mqListener }) => ({ ...acc, [name]: mqListener.matches }), {} ); + + // return result; + return unwrapSingleQuery(result); }; updateMatches = () => { const newMatches = this.getMatches(); - this.setState(() => ({ - matches: newMatches - }), this.onChange); + this.setState( + () => ({ + matches: newMatches + }), + this.onChange + ); }; initialize() { const targetWindow = this.props.targetWindow || window; invariant( - typeof targetWindow.matchMedia === 'function', - ' does not support `matchMedia`.' + typeof targetWindow.matchMedia === "function", + " does not support `matchMedia`." ); - const { queries } = this.props; + const queries = this.props.queries || wrapInQueryObject(this.props.query); this.queries = Object.keys(queries).map(name => { const query = queries[name]; const qs = typeof query !== "string" ? json2mq(query) : query; - const mqList = new MediaQueryList(targetWindow, qs, this.updateMatches); + const mqListener = new MediaQueryListener( + targetWindow, + qs, + this.updateMatches + ); - return { name, mqList }; + return { name, mqListener }; }); } @@ -107,35 +149,58 @@ class Media extends React.Component { } componentWillUnmount() { - this.queries.forEach(({ mqList }) => mqList.cancel()); + this.queries.forEach(({ mqListener }) => mqListener.cancel()); } render() { const { children, render } = this.props; const { matches } = this.state; - const isAnyMatches = Object.keys(matches).some(key => matches[key]); + const isAnyMatches = + typeof matches === "object" + ? Object.keys(matches).some(key => matches[key]) + : matches; return render ? isAnyMatches ? render(matches) : null : children - ? typeof children === 'function' - ? children(matches) - : // Preact defaults to empty children array - !Array.isArray(children) || children.length - ? isAnyMatches - ? // We have to check whether child is a composite component or not to decide should we - // provide `matches` as a prop or not - React.Children.only(children) && - typeof React.Children.only(children).type === "string" - ? React.Children.only(children) - : React.cloneElement(React.Children.only(children), { matches }) - : null - : null - : null; + ? typeof children === "function" + ? children(matches) + : !Array.isArray(children) || children.length // Preact defaults to empty children array + ? isAnyMatches + ? // We have to check whether child is a composite component or not to decide should we + // provide `matches` as a prop or not + React.Children.only(children) && + typeof React.Children.only(children).type === "string" + ? React.Children.only(children) + : React.cloneElement(React.Children.only(children), { matches }) + : null + : null + : null; + } +} + +/** + * Wraps a single query in an object. This is used to provide backward compatibility with + * the old `query` prop (as opposed to `queries`). If only a single query is passed, the object + * will be unpacked down the line, but this allows our internals to assume an object of queries + * at all times. + */ +function wrapInQueryObject(query) { + return { __DEFAULT__: query }; +} + +/** + * Unwraps an object of queries, if it was originally passed as a single query. + */ +function unwrapSingleQuery(queryObject) { + const queryNames = Object.keys(queryObject); + if (queryNames.length === 1 && queryNames[0] === "__DEFAULT__") { + return queryObject.__DEFAULT__; } + return queryObject; } export default Media; diff --git a/modules/MediaQueryList.js b/modules/MediaQueryListener.js similarity index 94% rename from modules/MediaQueryList.js rename to modules/MediaQueryListener.js index c8f30e8..29fa673 100644 --- a/modules/MediaQueryList.js +++ b/modules/MediaQueryListener.js @@ -1,4 +1,4 @@ -export default class MediaQueryList { +export default class MediaQueryListener { constructor(targetWindow, query, listener) { this.nativeMediaQueryList = targetWindow.matchMedia(query); this.active = true; diff --git a/modules/__tests__/Media-test.js b/modules/__tests__/Media-test.js index 1242ea9..0064218 100644 --- a/modules/__tests__/Media-test.js +++ b/modules/__tests__/Media-test.js @@ -1,6 +1,6 @@ import React from "react"; import Media from "../Media"; -import { renderStrict } from './utils'; +import { renderStrict } from "./utils"; const createMockMediaMatcher = matchesOrMapOfMatches => qs => ({ matches: @@ -25,7 +25,7 @@ describe("A in browser environment", () => { let node; beforeEach(() => { - node = document.createElement('div'); + node = document.createElement("div"); }); const queries = { @@ -38,6 +38,74 @@ describe("A in browser environment", () => { window.matchMedia = createMockMediaMatcher(true); }); + describe("and a child DOM element", () => { + it("should render child", () => { + const element = ( + +
matched
+
+ ); + + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("matched"); + }); + }); + }); + + describe("and a child component", () => { + it("should render child and provide matches as a prop", () => { + const Component = props => + props.matches === true && matched; + + const element = ( + + + + ); + + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("matched"); + }); + }); + }); + + describe("and a children function", () => { + it("should render its children function call result", () => { + const element = ( + + {matches => + matches === true ? children as a function : null + } + + ); + + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("children as a function"); + }); + }); + }); + + describe("and a render prop", () => { + it("should render `render` prop call result", () => { + const element = ( + matches === true && render prop} + /> + ); + + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("render prop"); + }); + }); + }); + }); + + describe("with multiple queries that match", () => { + beforeEach(() => { + window.matchMedia = createMockMediaMatcher(true); + }); + describe("and a child DOM element", () => { it("should render child", () => { const element = ( @@ -74,7 +142,9 @@ describe("A in browser environment", () => { const element = ( {matches => - (matches.sm && matches.lg) ? children as a function : null + matches.sm && matches.lg ? ( + children as a function + ) : null } ); @@ -103,7 +173,69 @@ describe("A in browser environment", () => { }); }); - describe('with a query that does not match', () => { + describe("with a query that does not match", () => { + beforeEach(() => { + window.matchMedia = createMockMediaMatcher(false); + }); + + describe("and a child DOM element", () => { + it("should not render anything", () => { + const element = ( + +
I am not rendered
+
+ ); + + renderStrict(element, node, () => { + expect(node.innerHTML || "").not.toMatch("I am not rendered"); + }); + }); + }); + + describe("and a child component", () => { + it("should not render anything", () => { + const Component = () => I am not rendered; + + const element = ( + + + + ); + + renderStrict(element, node, () => { + expect(node.innerHTML || "").not.toMatch("I am not rendered"); + }); + }); + }); + + describe("and a children function", () => { + it("should render children function call result", () => { + const element = ( + + {matches => matches === false && no matches at all} + + ); + + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("no matches at all"); + }); + }); + }); + + describe("and a render prop", () => { + it("should not call render prop at all", () => { + const render = jest.fn(); + + const element = ; + + renderStrict(element, node, () => { + expect(render).not.toBeCalled(); + }); + }); + }); + }); + + describe("with a multiple queries that do not match", () => { beforeEach(() => { window.matchMedia = createMockMediaMatcher(false); }); @@ -117,7 +249,7 @@ describe("A in browser environment", () => { ); renderStrict(element, node, () => { - expect(node.innerHTML || '').not.toMatch("I am not rendered"); + expect(node.innerHTML || "").not.toMatch("I am not rendered"); }); }); }); @@ -133,7 +265,7 @@ describe("A in browser environment", () => { ); renderStrict(element, node, () => { - expect(node.innerHTML || '').not.toMatch("I am not rendered"); + expect(node.innerHTML || "").not.toMatch("I am not rendered"); }); }); }); @@ -167,7 +299,7 @@ describe("A in browser environment", () => { }); }); - describe("with a query that partially match", () => { + describe("with queries that partially match", () => { const queries = { sm: "(max-width: 1000px)", lg: "(max-width: 2000px)" @@ -206,15 +338,13 @@ describe("A in browser environment", () => { {matches => matches.sm && - !matches.lg && yep, something definetly matched + !matches.lg && yep, something definitely matched } ); renderStrict(element, node, () => { - expect(node.innerHTML).toMatch( - "yep, something definetly matched" - ); + expect(node.innerHTML).toMatch("yep, something definitely matched"); }); }); }); @@ -244,7 +374,7 @@ describe("A in browser environment", () => { window.matchMedia = createMockMediaMatcher(true); }); - it('renders its child', () => { + it("renders its child", () => { const testWindow = { matchMedia: createMockMediaMatcher(false) }; @@ -260,8 +390,8 @@ describe("A in browser environment", () => { }); }); - describe('when a non-window prop is passed for targetWindow', () => { - it('errors with a useful message', () => { + describe("when a non-window prop is passed for targetWindow", () => { + it("errors with a useful message", () => { const notAWindow = {}; const element = ( @@ -272,17 +402,17 @@ describe("A in browser environment", () => { expect(() => { renderStrict(element, node, () => {}); - }).toThrow('does not support `matchMedia`'); + }).toThrow("does not support `matchMedia`"); }); }); }); - describe('when an onChange function is passed', () => { + describe("when an onChange function is passed", () => { beforeEach(() => { window.matchMedia = createMockMediaMatcher(true); }); - it('calls the function with the match result', () => { + it("calls the function with the match result", () => { const callback = jest.fn(); const element = ; @@ -297,14 +427,40 @@ describe("A in browser environment", () => { window.matchMedia = createMockMediaMatcher(false); }); - it("initially overwrites defaultMatches with matches from matchMedia", async () => { - const element = - {({ matches }) => matches ?
fully matched
:
not matched
} -
; + describe("for a single query", () => { + it("initially overwrites defaultMatches with matches from matchMedia", async () => { + const element = ( + + {matches => + matches === true ? ( +
fully matched
+ ) : ( +
not matched
+ ) + } +
+ ); + + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("not matched"); + }); + }); + }); + + describe("for multiple queries", () => { + it("initially overwrites defaultMatches with matches from matchMedia", async () => { + const element = ( + + {({ matches }) => + matches ?
fully matched
:
not matched
+ } +
+ ); - renderStrict(element, node, () => { - expect(node.innerHTML).toMatch('not matched'); + renderStrict(element, node, () => { + expect(node.innerHTML).toMatch("not matched"); + }); }); }); - }) + }); }); From 11792078661da9219cf7ee5fff25fb0ab4c20225 Mon Sep 17 00:00:00 2001 From: Edo Rivai Date: Sun, 17 Mar 2019 18:41:20 +0100 Subject: [PATCH 2/2] Implement SSR tests for backward-compat query prop --- modules/Media.js | 2 +- modules/__tests__/Media-ssr-test.js | 134 +++++++++++++++++++--------- 2 files changed, 92 insertions(+), 44 deletions(-) diff --git a/modules/Media.js b/modules/Media.js index abe9e2e..784adf8 100644 --- a/modules/Media.js +++ b/modules/Media.js @@ -57,7 +57,7 @@ class Media extends React.Component { if (typeof window !== "object") { // In case we're rendering on the server, apply the default matches let matches; - if (props.defaultMatches) { + if (props.defaultMatches !== undefined) { matches = props.defaultMatches; } else if (props.query) { matches = true; diff --git a/modules/__tests__/Media-ssr-test.js b/modules/__tests__/Media-ssr-test.js index 4e33ba8..955ebed 100644 --- a/modules/__tests__/Media-ssr-test.js +++ b/modules/__tests__/Media-ssr-test.js @@ -3,56 +3,104 @@ import React from "react"; import Media from "../Media"; -import { serverRenderStrict } from './utils'; +import { serverRenderStrict } from "./utils"; describe("A in server environment", () => { - const queries = { - sm: "(max-width: 1000px)", - lg: "(max-width: 2000px)", - xl: "(max-width: 3000px)" - }; - - describe("when no default matches prop provided", () => { - it("should render its children as if all queries are matching", () => { - const element = ( - - {matches => ( - (matches.sm && matches.lg && matches.xl) - ? All matches, render! - : null - )} - - ); - - const result = serverRenderStrict(element); - - expect(result).toBe("All matches, render!"); + describe("and a single query is defined", () => { + const query = "(max-width: 1000px)"; + + describe("when no default matches prop provided", () => { + it("should render its children as if the query matches", () => { + const element = ( + + {matches => + matches === true ? Matches, render! : null + } + + ); + + const result = serverRenderStrict(element); + + expect(result).toBe("Matches, render!"); + }); + }); + + describe("when default matches prop provided", () => { + it("should render its children according to the provided defaultMatches", () => { + const render = matches => (matches === true ? matches : null); + + const matched = ( + + {render} + + ); + + const matchedResult = serverRenderStrict(matched); + + expect(matchedResult).toBe("matches"); + + const notMatched = ( + + {render} + + ); + + const notMatchedResult = serverRenderStrict(notMatched); + + expect(notMatchedResult).toBe(""); + }); }); }); - describe("when default matches prop provided", () => { - const defaultMatches = { - sm: true, - lg: false, - xl: false + describe("and multiple queries are defined", () => { + const queries = { + sm: "(max-width: 1000px)", + lg: "(max-width: 2000px)", + xl: "(max-width: 3000px)" }; - it("should render its children according to the provided defaultMatches", () => { - const element = ( - - {matches => ( -
- {matches.sm && small} - {matches.lg && large} - {matches.xl && extra large} -
- )} -
- ); - - const result = serverRenderStrict(element); - - expect(result).toBe("
small
"); + describe("when no default matches prop provided", () => { + it("should render its children as if all queries are matching", () => { + const element = ( + + {matches => + matches.sm && matches.lg && matches.xl ? ( + All matches, render! + ) : null + } + + ); + + const result = serverRenderStrict(element); + + expect(result).toBe("All matches, render!"); + }); + }); + + describe("when default matches prop provided", () => { + const defaultMatches = { + sm: true, + lg: false, + xl: false + }; + + it("should render its children according to the provided defaultMatches", () => { + const element = ( + + {matches => ( +
+ {matches.sm && small} + {matches.lg && large} + {matches.xl && extra large} +
+ )} +
+ ); + + const result = serverRenderStrict(element); + + expect(result).toBe("
small
"); + }); }); }); });