Skip to content

Commit

Permalink
Merge pull request #1 from teamplanes/feature/prevent-deep-nesting
Browse files Browse the repository at this point in the history
feat: working deep nesting prevention with initial passing test
  • Loading branch information
kirkness committed Oct 20, 2020
2 parents a4d8c7b + 5ab0866 commit 3b8dc3e
Show file tree
Hide file tree
Showing 4 changed files with 214 additions and 66 deletions.
33 changes: 33 additions & 0 deletions src/Breakpoints.ts
Expand Up @@ -134,6 +134,39 @@ export class Breakpoints<BreakpointKey extends string> {
}) as BreakpointKey | undefined
}

public toVisibleAtBreakpointSet(breakpointProps: MediaBreakpointProps) {
breakpointProps = this._normalizeProps(breakpointProps)
if (breakpointProps.lessThan) {
const breakpointIndex = this.sortedBreakpoints.findIndex(
bp => bp === breakpointProps.lessThan
)
return this.sortedBreakpoints.slice(0, breakpointIndex)
} else if (breakpointProps.greaterThan) {
const breakpointIndex = this.sortedBreakpoints.findIndex(
bp => bp === breakpointProps.greaterThan
)
return this.sortedBreakpoints.slice(breakpointIndex + 1)
} else if (breakpointProps.greaterThanOrEqual) {
const breakpointIndex = this.sortedBreakpoints.findIndex(
bp => bp === breakpointProps.greaterThanOrEqual
)
return this.sortedBreakpoints.slice(breakpointIndex)
} else if (breakpointProps.between) {
const between = breakpointProps.between
const fromBreakpointIndex = this.sortedBreakpoints.findIndex(
bp => bp === between[0]
)
const toBreakpointIndex = this.sortedBreakpoints.findIndex(
bp => bp === between[1]
)
return this.sortedBreakpoints.slice(
fromBreakpointIndex,
toBreakpointIndex
)
}
return []
}

public toRuleSets(keys = Breakpoints.validKeys()) {
const selectedMediaQueries = keys.reduce(
(mediaQueries, query) => {
Expand Down
164 changes: 98 additions & 66 deletions src/Media.tsx
Expand Up @@ -325,6 +325,12 @@ export function createMedia<
>({})
MediaContext.displayName = "Media.Context"

const MediaParentContext = React.createContext<{
hasParentMedia: boolean
breakpointProps: MediaBreakpointProps<BreakpointKey>
}>({ hasParentMedia: false, breakpointProps: {} })
MediaContext.displayName = "MediaParent.Context"

const MediaContextProvider: React.SFC<
MediaContextProviderProps<BreakpointKey | Interaction>
> = ({ disableDynamicMediaQueries, onlyMatch, children }) => {
Expand Down Expand Up @@ -380,76 +386,102 @@ export function createMedia<
className: "",
}

static contextType = MediaParentContext

render() {
const props = this.props
const {
children,
className: passedClassName,
interaction,
...breakpointProps
} = props
return (
<MediaContext.Consumer>
{({ onlyMatch } = {}) => {
let className: string | null
const {
children,
className: passedClassName,
interaction,
...breakpointProps
} = props
if (props.interaction) {
className = createClassName("interaction", props.interaction)
} else {
if (props.at) {
const largestBreakpoint =
mediaQueries.breakpoints.largestBreakpoint
if (props.at === largestBreakpoint) {
// TODO: We should look into making React’s __DEV__ available
// and have webpack completely compile these away.
let ownerName = null
try {
const owner = (this as any)._reactInternalFiber._debugOwner
.type
ownerName = owner.displayName || owner.name
} catch (err) {
// no-op
}

console.warn(
"[@artsy/fresnel] " +
"`at` is being used with the largest breakpoint. " +
"Consider using `<Media greaterThanOrEqual=" +
`"${largestBreakpoint}">\` to account for future ` +
`breakpoint definitions outside of this range.${
ownerName
? ` It is being used in the ${ownerName} component.`
: ""
}`
)
}
}

const type = propKey(breakpointProps)
const breakpoint = breakpointProps[type]!
className = createClassName(type, breakpoint)
}

const renderChildren =
onlyMatch === undefined ||
mediaQueries.shouldRenderMediaQuery(
{ ...breakpointProps, interaction },
onlyMatch
)

if (props.children instanceof Function) {
return props.children(className, renderChildren)
} else {
return (
<div
className={`fresnel-container ${className} ${passedClassName}`}
suppressHydrationWarning={!renderChildren}
>
{renderChildren ? props.children : null}
</div>
)
}
<MediaParentContext.Consumer>
{mediaParentContext => {
return (
<MediaParentContext.Provider
value={{ hasParentMedia: true, breakpointProps }}
>
<MediaContext.Consumer>
{({ onlyMatch } = {}) => {
let className: string | null
if (props.interaction) {
className = createClassName(
"interaction",
props.interaction
)
} else {
if (props.at) {
const largestBreakpoint =
mediaQueries.breakpoints.largestBreakpoint
if (props.at === largestBreakpoint) {
// TODO: We should look into making React’s __DEV__ available
// and have webpack completely compile these away.
let ownerName = null
try {
const owner = (this as any)._reactInternalFiber
._debugOwner.type
ownerName = owner.displayName || owner.name
} catch (err) {
// no-op
}

console.warn(
"[@artsy/fresnel] " +
"`at` is being used with the largest breakpoint. " +
"Consider using `<Media greaterThanOrEqual=" +
`"${largestBreakpoint}">\` to account for future ` +
`breakpoint definitions outside of this range.${
ownerName
? ` It is being used in the ${ownerName} component.`
: ""
}`
)
}
}

const type = propKey(breakpointProps)
const breakpoint = breakpointProps[type]!
className = createClassName(type, breakpoint)
}

const doesMatchParent =
!mediaParentContext.hasParentMedia ||
intersection(
mediaQueries.breakpoints.toVisibleAtBreakpointSet(
mediaParentContext.breakpointProps
),
mediaQueries.breakpoints.toVisibleAtBreakpointSet(
breakpointProps
)
).length > 0
const renderChildren =
doesMatchParent &&
(onlyMatch === undefined ||
mediaQueries.shouldRenderMediaQuery(
{ ...breakpointProps, interaction },
onlyMatch
))

if (props.children instanceof Function) {
return props.children(className, renderChildren)
} else {
return (
<div
className={`fresnel-container ${className} ${passedClassName}`}
suppressHydrationWarning={!renderChildren}
>
{renderChildren ? props.children : null}
</div>
)
}
}}
</MediaContext.Consumer>
</MediaParentContext.Provider>
)
}}
</MediaContext.Consumer>
</MediaParentContext.Consumer>
)
}
}
Expand Down
43 changes: 43 additions & 0 deletions src/__test__/Breakpoints.test.tsx
@@ -0,0 +1,43 @@
import { Breakpoints } from "../Breakpoints"

const config = {
"extra-small": 0,
small: 768,
medium: 1024,
large: 1120,
}

const breakpoint = new Breakpoints(config)

describe("Breakpoints", () => {
describe.only("toVisibleAtBreakpointSet", () => {
it("returns correct values for greaterThan", () => {
const breakpoints = breakpoint.toVisibleAtBreakpointSet({
greaterThan: "small",
})
expect(breakpoints).toEqual(["medium", "large"])
})
it("returns correct values for greaterThanOrEqual", () => {
const breakpoints = breakpoint.toVisibleAtBreakpointSet({
greaterThanOrEqual: "small",
})
expect(breakpoints).toEqual(["small", "medium", "large"])
})
it("returns correct values for lessThan", () => {
const breakpoints = breakpoint.toVisibleAtBreakpointSet({
lessThan: "small",
})
expect(breakpoints).toEqual(["extra-small"])
})
it("returns correct values for at", () => {
const breakpoints = breakpoint.toVisibleAtBreakpointSet({ at: "small" })
expect(breakpoints).toEqual(["small"])
})
it("returns correct values for between", () => {
const breakpoints = breakpoint.toVisibleAtBreakpointSet({
between: ["extra-small", "medium"],
})
expect(breakpoints).toEqual(["extra-small", "small"])
})
})
})
40 changes: 40 additions & 0 deletions src/__test__/Media.test.tsx
Expand Up @@ -530,6 +530,46 @@ describe("Media", () => {
})
})

describe("prevent nested unnecessary renders", () => {
it("only renders one element when Media is nested within Media", () => {
const query = renderer.create(
<MediaContextProvider>
<Media at="extra-small">
<span className="extra-small" />
</Media>

<Media at="medium">
<Media at="extra-small">
<span className="extra-small" />
</Media>
<Media at="medium">
<span className="medium" />
</Media>
<Media at="large">
<span className="large" />
</Media>
</Media>

<Media at="large">
<span className="large" />
</Media>
</MediaContextProvider>
)

expect(
query.root.findAllByProps({ className: "extra-small" }, { deep: true })
.length
).toBe(1)
expect(
query.root.findAllByProps({ className: "large" }, { deep: true }).length
).toBe(1)
expect(
query.root.findAllByProps({ className: "medium" }, { deep: true })
.length
).toBe(1)
})
})

// TODO: This actually doesn’t make sense, I think, because if the user
// decides to not use a provider they are opting for rendering all
// variants. We just need to make sure to document this well.
Expand Down

0 comments on commit 3b8dc3e

Please sign in to comment.