Add ability to mount()/shallow() a node inside a wrapping component #1960

minznerjosh · 2019-01-02T22:03:10Z

Overview

This PR adds an API for mount()ing a node inside of a custom wrappingComponent.

import { Provider } from 'react-redux';
import { Router } from 'react-router';
import store from './my/app/store';
import mockStore from './my/app/mockStore';

function MyProvider(props) {
  const { children, customStore } = props;

  return (
    <Provider store={customStore || store}>
      <Router>
        {children}
      </Router>
    </Provider>
  );
}

const wrapper = mount(<MyComponent />, {
  wrappingComponent: MyProvider,
});

It is also possible to get a ReactWrapper around the wrappingComponent:

const provider = wrapper.getWrappingComponent();
provider.setProps({ customStore: mockStore });
wrapper.update(); // update root wrapper after wrappingComponent props changed

Related to #1958.

…ootFinder`, `wrapWithWrappingComponent`, `getWrappingComponentMountRenderer`; add `RootFinder`

…ithWrappingComponent`

minznerjosh · 2019-01-02T22:03:41Z

docs/api/ReactWrapper/setProviders.md

+
+- `.setProviders()` can only be used on a wrapper that was initially created with a call to `mount()`
+- `.setProviders()` is only supported in React >= 16.3
+- `.setProviders()` requires `enzyme-adapter-react-16@>=1.8` or `enzyme-adapter-react-16.3@>=1.5`


These versions will need to be updated depending on when/if this makes it in.

Let's remove version numbers entirely; there's no need to specify it here.

ljharb · 2019-01-02T22:05:41Z

Adding createContext support in any form is a major effort; I want to set expectations early that this PR may not make it in at all, and that support is required to be identical for both mount and shallow.

ljharb

Nothing react-specific can go in enzyme itself, only in adapters - as such, a new adapter interface for createContext would have to be added to make this work. Additionally, we'd have to retain compatibility between old enzyme and new adapters, new enzyme and old adapters, and new enzyme and new adapters.

ljharb · 2019-01-02T22:06:51Z

docs/api/ReactWrapper/setProviders.md

+
+- `.setProviders()` can only be used on a wrapper that was initially created with a call to `mount()`
+- `.setProviders()` is only supported in React >= 16.3
+- `.setProviders()` requires `enzyme-adapter-react-16@>=1.8` or `enzyme-adapter-react-16.3@>=1.5`


Let's remove version numbers entirely; there's no need to specify it here.

ljharb · 2019-01-02T22:07:22Z

docs/api/ReactWrapper/setProviders.md

+#### Common Gotchas
+
+- `.setProviders()` can only be used on a wrapper that was initially created with a call to `mount()`
+- `.setProviders()` is only supported in React >= 16.3


mount itself has no direct tie to React; it should be able to work as long as the adapter supports it (which might not be react at all)

Sounds good. Is there any prior art for what to do in a case where an adapter does not support a feature? For example, I'm assuming we'll never want to add support for this API to adapters that handle react@<16.3.

ljharb · 2019-01-02T22:08:00Z

packages/enzyme-adapter-utils/src/createMountWrapper.jsx

+  reversed.reverse();
+
+  return reversed.reduce((childElement, parentElement) => (
+    React.cloneElement(parentElement, null, childElement)


cloning elements is very slow and should be avoided. why is it needed here?

The goal is to convert the provider elements the user passes into a tree:

So this:

mount(<MyComponent />, { providers: [ <ContextA.Provider value="A" />, <ContextB.Provider value="B" />, <ContextC.Provider value="C" />, ], });

Is rendered as:

<ContextA.Provider value="A"> <ContextB.Provider value="B"> <ContextC.Provider value="C"> <MyComponent /> </ContextC.Provider> </ContextB.Provider> </ContextA.Provider>

And I figured cloneElement() was the most idiomatic way to do this. I'm also working under the assumption that mutating the providers the user passes in would be a bad idea. Would a more manual

// not sure if this will actually work { ...parentElement, props: { ...parentElement.props, children: childElement, }, }

be preferable, or is it the copying itself that is too slow to be acceptable?

ljharb · 2019-01-02T22:09:30Z

packages/enzyme-adapter-utils/src/createMountWrapper.jsx

+
+      this.setState({
+        providers: currentProviders.filter(provider => (
+          !providerTypes.includes(provider.type)


this API means there's never any way to remove a provider, only to replace existing ones or add new ones.

Separately, providers being an array implies that the order matters - why and how? If it matters because it's nested into a tree, then it seems much simpler to only ever allow a single provider element, and come up with an alternative solution that lets the user determine their own nesting yet also indicate where children should be rendered.

I think you can effectively remove a provider by doing

wrapper.setProviders([<Context.Provider value={undefined} />]);

As for order mattering, it doesn't. I used an array because objects can only have strings as keys and it seemed wrong to require configuration options that do not have a literal syntax (like Set or Map.) I wrote a bit about my thought process when coming up with the API in the PR description.)

But I'm open to an entirely different API if you have some ideas. 😊

I'm a bit confused why multiple providers are needed, since you can do:

<Provider1> <Provider2 /> </Provider1>

and provide a single provider

Ah got it! I was a bit confused by what you meant by "a single provider". I was thinking about the new context API in terms of individual values (as is the case with the old API), but perhaps that was me trying to make the new API into something that it is not.

The way you compose multiple contexts in the new React API is by nesting components—perhaps the enzyme API should reflect that, as you've pointed out! So, what do you think about this:

mount(<MyComponent />, { providers: ( <ContextA.Provider value="A"> <ContextB.Provider value="B"> <ContextC.Provider value="C" /> </ContextB.Provider> </ContextA.Provier> ), });

And then changing via context by

wrapper.setProviders( <ContextA.Provider value="A"> <ContextB.Provider value="foo"> <ContextC.Provider value="C" /> </ContextB.Provider> </ContextA.Provier> );

My only concern is that this API makes it a bit challenging to change just a single provider's value without having to re-specify the entire tree. I think it will be common to be dealing with a bunch of different contexts from different libraries, all of which are required for a component to render. But each individual spec will deal with the how the component responds to just one of those contexts changing. So, what do you think about (in addition to the .setProviders() API) having an API like this:

wrapper.updateProvider(<ContextB.Provider value="foo" />);

Which would update just that provider, wherever it is in the tree, and throw an error if the provider is not already in the tree.

Also, I don't feel strongly about any of these names if you'd like them to be different. I am leaning towards keeping the plural "providers" because I think it makes it a bit more clear that the element can have many nested providers, though I do realize you are only passing a single react element.

Re-specifying the entire tree is already the case if you want to change a child of the element you wrapped, for example - i'm also not clear on the use case of changing providers after initially wrapping.

Maybe instead of this feature being specific to createContext providers, it could be more general - something like, "wrapWith". A single render prop function that gets passed a child, and wraps it as needed.

Example:

const wrapper = mount(<Foo />, { wrapWith(root) { return ( <ContextA.Provider value="A"> <ContextB.Provider value="foo"> <ContextC.Provider value="C"> {root} </ContextC.Provider> </ContextB.Provider> </ContextA.Provider> ); }, });

The API could throw if your wrapWith function failed to render the child (solo, ie, with no siblings), and then wrapper in this case would be <Foo />, not any of the providers, but they'd still be exercised in the rendering path.

I think the use-case is something like this:

Imagine you have a react-flag-like library and some future version of react-redux that uses the new context API. You'd render your component like this (using the wrapWith API):

// my app's feature flags const flags = { foo: false, bar: false, }; const wrapper = mount(<Foo />, { wrapWith: root => ( <FlagProvider value={flags}> <ReduxProvider value={store}>{root}</ReduxProvider> </FlagProvider> ), });

And now we want to test that some side-effect in <Foo />'s componentDidUpdate gets called when the feature flag changes. We need some way of changing <FlagProvider />'s value, and it would be nice if we didn't have to re-setup the <ReduxProvider /> when we do that.

Also, if our component starts depending on a new provider in the future, it'd be really nice to only have to add it to the options we pass enzyme, and not every place where we're just changing a single provider's value.

I'm guessing the API for changing the wrapWith function could be something like

wrapper.setWrapWith(root => ( <FlagProvider value={{ ...flags, bar: true }}> <ReduxProvider value={store}>{root}</ReduxProvider> </FlagProvider> ));

But that doesn't address my concern about having to respecify the entire tree. One solution that comes to mind (just spitballing here) is making wrapWith a component:

const wrapper = mount(<Foo />, { wrapper: { component: (props) => { const { flags, store, children } = props; return ( <FlagProvider value={{ ...flags, bar: true }}> <ReduxProvider value={store}>{children}</ReduxProvider> </FlagProvider> ); }, props: { store: createStore, flags }, }, });

And then having an API that lets you set the wrapper component's props:

wrapper.setWrapperProps({ flags: { ...flags, foo: true, }, });

(wrapWith seemed like a better name for a function than a component so I changed it to just wrapper; but it also seems a bit confusing because wrapper is the conventional name for a ReactWrapper instance.)

(We can bikeshed the names later)

OK, so you're suggesting the user be able to provide a "wrapping component", and provide explicit setProps/setState access on it - what about instead, wrapper.getWrappingComponent(), that returned a normal enzyme wrapper around that component - providing you with the same APIs?

I like that idea a lot! I'll implement and then drop you a comment here so we can discuss. I really appreciate the time you've taken to work through this with me. Thank you!

minznerjosh · 2019-01-02T22:11:00Z

@ljharb I figured that'd be the case! Just wanted to get something up here to get a conversation started.

packages/enzyme/src/ReactWrapper.js

minznerjosh · 2019-01-02T23:01:18Z

@ljharb So, I'm sure you've noticed that this PR does not add support for shallow(). I'll give that a whirl, but I think it'd be best to hammer out an API before I do. Thanks again for such a speedy first review! Cheers!

packages/enzyme-test-suite/package.json

packages/enzyme-adapter-utils/src/RootFinder.jsx

minznerjosh · 2019-01-04T16:31:58Z

packages/enzyme-adapter-utils/src/createMountWrapper.jsx

+      if (WrappingComponent) {
+        return (
+          <WrappingComponent {...wrappingComponentProps}>
+            <RootFinder ref={this.setRootFinderInstance}>{component}</RootFinder>


We need a reliable way to get the node that was passed as the first argument to mount() and this seemed like the simplest way. Is it acceptable? Are there going to be performance implications of using a ref?

we should avoid using ref, yes.

minznerjosh · 2019-01-04T16:35:00Z

packages/enzyme-test-suite/test/ReactWrapper-spec.jsx

@@ -164,6 +164,156 @@ describeWithDOM('mount', () => {
      expect(() => wrapper.state('key')).to.throw('ReactWrapper::state("key") requires that `state` not be `null` or `undefined`');
    });

+    describeIf(is('>= 0.14') ,'wrappingComponent', () => {


There's no reason this feature couldn't work in react 13, but the tests don't because react 13 uses owner-based context instead of parent-based. Is it worth coming up with a different test methodology for react 0.13, or are we okay with not supporting this feature in the 0.13 adapter? (I'm not sure how useful this feature is without parent-based context.)

ideally we'll solve it for react 13 as well; it's probably OK if we defer for now as long as we're confident this enzyme API won't need to change to support it.

minznerjosh · 2019-01-04T16:36:28Z

packages/enzyme/src/ReactWrapper.js

      privateSet(this, ROOT, root);
      privateSetNodes(this, nodes);
      privateSet(this, ROOT_NODES, root[NODES]);
    }
+    privateSet(this, UNRENDERED, nodes);


UNRENDERED needs to be set to call .setProps() on the wrappingComponent. It didn't seem like there was any harm in setting it to nodes regardless of !root. (All the tests still passed.)

minznerjosh · 2019-01-04T16:40:24Z

packages/enzyme/src/ReactWrapper.js

+    const node = this[WRAPPING_COMPONENT_RENDERER].getNode();
+    const wrapper = this.wrap(node);
+    privateSet(wrapper, ROOT, wrapper);
+    privateSet(wrapper, RENDERER, this[WRAPPING_COMPONENT_RENDERER]);


This is a bit hacky. Is it acceptable? We don't want this new ReactWrapper to render itself, but we do want it to be a root (so we can .setProps() on it.)

We could also decide to not support .setProps() and make it a non-root. In that case, we could just get by using .setState() to change the context.

minznerjosh · 2019-01-04T17:14:07Z

@ljharb

TL;DR;

I implemented the API we discussed for mount()
Some of the things I did might be a bit too hacky
Things could be simplified and made less-hacky if .getWrappingComponent() did not return a root
I'm not sure how much sense this API makes for shallow(); any possibility of moving forward without shallow()?

I took a stab at implementing the API we discussed for mount(). Because I wanted .setProps() to work, I made the .getWrappingComponent() wrapper a root (of sorts.) This led me to do some things that might be a bit questionable. Because we can now .setState() non-roots, perhaps we can live with .getWrappingComponent() not returning a root. That, however, would create complications around .getWrappingComponent().update(). (It would update the root, not the wrappingComponent.)

I don't use shallow() much in my day-to-day, but as I started digging into how it works, I started to doubt how useful this feature would be for shallow(). The only reason I can think of for the wrappingComponent feature to exist is for users can set up context (new or legacy) without changing the root wrapper. I think this is a need that arises from integration-style testing, whereas shallow() is strictly for unit-style tests. If we were to just shallow-render the wrappingComponent and then effectively .dive() to the root, the effect would be as if we didn't have a wrappingComponent at all.

Perhaps this is because, as others have pointed out, .dive() doesn't currently call .getChildContext() or look at .contextTypes. I imagine we could do something where we dive through every component in the wrappingComponent, collect its context, and then pass that context when we shallow render the root node, but that seems a bit complicated and would only work for legacy context, not the new context API. Perhaps we could do something similar with the new context Provider/Consumer, though.

But I still doubt how much sense wrappingComponent makes as a shallow() option. If shallow() exists to isolate components from each other, it doesn't seem intuitive to me to support wrapping your component in a wrappingComponent because shallow rendering, by definition, will shield the root component from the effects of wrappingComponent.

So, my question is, is there any possibility you'd be open to this being a mount()-only feature? If shallow() support really is required, what should the behavior for it be? If we decide that the wrappingComponent feature only exists for providing context, then there would be a lot of work required in shallow() to support that. If wrappingComponent exists literally just to wrap the enzyme root in another component, and follow the existing behavior for doing that with mount()/shallow(), then I don't think it's a useful feature for shallow().

ljharb · 2019-01-04T21:28:59Z

shallow needs providers too; I can't see how it wouldn't make just as much sense as for mount. (and i'd suggest using shallow always, and only resorting to mount when necessary, but that's a more subjective discussion :-) ) shallow isn't just for "unit-style tests" - everything is an integration test, and shallow with a wrapping component would be for testing what's shallow-wrapped, but with certain context values and certain state-populating lifecycle methods to be called.

It's a serious issue that .dive() doesn't handle context properly, and my hope is that this API can lead us to a path that would allow that bug to be fixed.

minznerjosh · 2019-01-04T21:45:26Z

So, is this the strategy you're looking for in shallow()?

Do a full render of the wrappingComponent, but without a dependency on the DOM, stopping when we reach the node passed to shallow(node)
Collect all the context values that were set in that full render of wrappingComponent (both new and legacy)
Shallow-render the node, passing in all the context we collected in step 2 (using some new functionality that doesn't exist yet for new context)

ljharb · 2019-01-04T21:46:59Z

Yep! sounds right to me (where "full render" is actually multiple shallow renders and dives)

minznerjosh · 2019-01-04T21:53:13Z

Yup yup. Okay, in that case, I think I'm going to hit the pause button on this PR and try to add support for dive()ing <Consumer />s and <Provider />s in a separate PR. That seems like a separate issue that should be fixed independently, and fixing that issue should lay the foundation for implementing this feature.

ljharb · 2019-01-04T22:04:40Z

That sounds like an awesome plan!

minznerjosh · 2019-02-26T21:00:51Z

@ljharb updated!

minznerjosh · 2019-03-04T13:58:37Z

Whoops, I can see I broke some stuff. Will fix ASAP!

ljharb · 2019-03-13T07:00:34Z

ping :-) any update? i'm anxious to get this one in.

minznerjosh · 2019-03-13T14:09:06Z

@ljharb updated! The build succeeded!

minznerjosh · 2019-03-23T10:27:57Z

@ljharb think this could be merged soon?

…nent (#1960) Merge pull request #1960 from trialspark/josh__mount-new-context Related to #1958.

- [new] add `isCustomComponent` (#1960) - [dev deps] update `eslint`, `semver`, `rimraf`, `react-is`, `html-element-map`, `chai`, `eslint-plugin-mocha` - [build] include source maps

- [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [deps] update `react-is` - [dev deps] update `eslint`, `semver`, `rimraf`, `react-is`, `html-element-map`, `chai`, `eslint-plugin-mocha` - [build] include source maps

- [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [deps] update `react-is` - [dev deps] update `eslint` - [build] include source maps

- [new] support shallow rendering `createContext()` providers and consumers - add `isContextConsumer`, `getProviderFromConsumer` (#1966) - [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [refactor] use `react-is` predicates more - [deps] update `react-is` - [dev deps] update `eslint` - [build] include source maps

- [new] Add support for wrapping `Profiler` (#2055) - [new] support shallow rendering `createContext()` providers and consumers - add `isContextConsumer`, `getProviderFromConsumer` (#1966) - [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [new] add `getDerivedStateFromError` support (#2036) - [fix] avoid invariant violation in provider (#2083) - [fix] properly fix finding memo(SFC) components (#2081) - [fix] properly render memoized SFCs - [fix] `shallow`: avoid wrapping component for hooks - [deps] update `react-is` - [dev deps] update `eslint` - [refactor] use `react-is` predicates more - [build] include source maps

nelsonpecora · 2019-05-23T16:16:20Z

Hi folks, I just ran into this issue and I'm so happy someone's already solved it! From the last comment, does this mean we'll be able to use this feature when the next version of enzyme is released?

ljharb · 2019-06-04T00:58:13Z

v3.10.0 has now been released.

minznerjosh added 5 commits January 2, 2019 16:48

[enzyme-adapter-utils] [new] Utils add findElement, `getNodeFromR…

87b0b39

…ootFinder`, `wrapWithWrappingComponent`, `getWrappingComponentMountRenderer`; add `RootFinder`

[enzyme-adapter-react-*] [new] add isCustomComponent

c89f337

[enzyme-adapter-react-{14,15.4,15,16.1,16.2,16.3,16] [new] add `wrapW…

76ca983

…ithWrappingComponent`

[New] Utils: add isCustomComponent

26f5492

[New] mount/shallow: add getWrappingComponent

cc3db79

minznerjosh commented Jan 2, 2019

View reviewed changes

ljharb requested changes Jan 2, 2019

View reviewed changes

minznerjosh commented Jan 2, 2019

View reviewed changes

packages/enzyme/src/ReactWrapper.js Outdated Show resolved Hide resolved

ljharb reviewed Jan 3, 2019

View reviewed changes

packages/enzyme-test-suite/package.json Outdated Show resolved Hide resolved

minznerjosh force-pushed the josh__mount-new-context branch from 02c9040 to e675ca9 Compare January 4, 2019 16:25

minznerjosh changed the title ~~Add a mount() API for manipulating React's new context~~ Add ability to mount() a node inside a wrapping component Jan 4, 2019

minznerjosh commented Jan 4, 2019

View reviewed changes

packages/enzyme-adapter-utils/src/RootFinder.jsx Show resolved Hide resolved

minznerjosh commented Jan 4, 2019

View reviewed changes

This was referenced Jan 5, 2019

Add ability to dive() and shallow-render-as-root Consumers and Providers #1966

Merged

How should enzyme support the createContext() API #1973

Closed

minznerjosh changed the title ~~Add ability to mount() a node inside a wrapping component~~ Add ability to mount()/shallow() a node inside a wrapping component Jan 12, 2019

minznerjosh mentioned this pull request Jan 12, 2019

Potential issues with store as a prop and shallow test rendering in v6 reduxjs/react-redux#1161

Closed

ljharb added this to Context in React 16 Jan 23, 2019

ljharb mentioned this pull request Feb 26, 2019

Redux Provider and set context #1013

Closed

minznerjosh force-pushed the josh__mount-new-context branch from 34e2c84 to b26d5f6 Compare February 26, 2019 20:53

ljharb force-pushed the josh__mount-new-context branch from b26d5f6 to 5c15a18 Compare February 26, 2019 23:31

minznerjosh force-pushed the josh__mount-new-context branch 2 times, most recently from 7aaa70b to 46e9853 Compare March 13, 2019 13:23

minznerjosh force-pushed the josh__mount-new-context branch from 46e9853 to 26a34ec Compare March 18, 2019 12:29

ljharb force-pushed the josh__mount-new-context branch from 26a34ec to edf5fc4 Compare March 24, 2019 21:06

OzzieOrca mentioned this pull request Mar 30, 2019

MHP-2233: Bring in react-native-testing-library and add utility funct… CruGlobal/missionhub-react-native#891

Closed

ljharb force-pushed the josh__mount-new-context branch from edf5fc4 to 1ca3dc1 Compare April 2, 2019 02:40

ljharb approved these changes Apr 2, 2019

View reviewed changes

ljharb force-pushed the josh__mount-new-context branch 2 times, most recently from 1de841b to cc3db79 Compare April 2, 2019 17:03

ljharb merged commit cc3db79 into enzymejs:master Apr 2, 2019

ljharb added a commit that referenced this pull request Apr 2, 2019

[New] Add ability to mount()/shallow() a node inside a wrapping compo…

a5edaf1

…nent (#1960) Merge pull request #1960 from trialspark/josh__mount-new-context Related to #1958.

minznerjosh deleted the josh__mount-new-context branch April 2, 2019 17:27

ljharb added a commit that referenced this pull request Apr 6, 2019

[enzyme-adapter-react-13] v1.4.0

8e24b1f

- [new] add `isCustomComponent` (#1960) - [dev deps] update `eslint`, `semver`, `rimraf`, `react-is`, `html-element-map`, `chai`, `eslint-plugin-mocha` - [build] include source maps

ljharb added a commit that referenced this pull request Apr 6, 2019

[enzyme-adapter-react-{16.1,16.2}] v1.6.0

ce36c19

- [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [deps] update `react-is` - [dev deps] update `eslint` - [build] include source maps

ljharb mentioned this pull request Apr 6, 2019

[Tests] add hooks tests #2008

Merged

7 tasks

kumar303 mentioned this pull request Apr 26, 2019

Fix infinite loops in compare page mozilla/addons-code-manager#639

Merged

JustinLex mentioned this pull request Jul 8, 2019

Theming section in README.md should mention the new wrappingComponent API in Enzyme styled-components/jest-styled-components#256

Open

Add ability to mount()/shallow() a node inside a wrapping component #1960

Add ability to mount()/shallow() a node inside a wrapping component #1960

Conversation

minznerjosh commented Jan 2, 2019 • edited by ljharb

Overview

minznerjosh Jan 2, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb commented Jan 2, 2019

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minznerjosh commented Jan 2, 2019

minznerjosh commented Jan 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minznerjosh commented Jan 4, 2019

ljharb commented Jan 4, 2019

minznerjosh commented Jan 4, 2019

ljharb commented Jan 4, 2019

minznerjosh commented Jan 4, 2019

ljharb commented Jan 4, 2019

minznerjosh commented Feb 26, 2019

minznerjosh commented Mar 4, 2019

ljharb commented Mar 13, 2019

minznerjosh commented Mar 13, 2019

minznerjosh commented Mar 23, 2019

nelsonpecora commented May 23, 2019

ljharb commented Jun 4, 2019

minznerjosh commented Jan 2, 2019 •

edited by ljharb

minznerjosh Jan 2, 2019 •

edited