Skip to content

react: Allow changing instance in uppy prop #1247

Merged
goto-bus-stop merged 2 commits intomasterfrom
feature/react-uppy-change
Jan 28, 2019
Merged

react: Allow changing instance in uppy prop #1247
goto-bus-stop merged 2 commits intomasterfrom
feature/react-uppy-change

Conversation

@goto-bus-stop
Copy link
Contributor

When changing the Uppy instance, the components will uninstall the UI plugin from the old instance, and install it to the new instance.

One other thing that I wanted to do is warn when you create the Uppy instance in the render() method. that becomes harder when this use case is supported. If we didn't support changing instances, we could simply throw an error if the uppy prop ever changes. If we support changing instances, I'm not sure how we'd detect Uppy instances being created in render().

@arturi
Copy link
Contributor

arturi commented Jan 22, 2019

👍

Should we make a note in the docs, in bold, that uppy shouldn’t be initialized in render? Issues about this arise quite often: #1243.

@goto-bus-stop
Copy link
Contributor Author

Maybe we can also provide a Component to do something like

<UppyProvider
  initialize={uppy => {
    uppy.use(Transloadit, {})
  }}
  render={uppy => (
    <MyComponent uppy={uppy} />
  )}
/>

to manage the lifecycle of an Uppy instance.

Or a withUppy HOC like

const enhance = withUppy(uppy => {
  uppy.use(Transloadit, {})
})
const MyComponent = ({ uppy }) => (
  <Dashboard uppy={uppy} />
)
const MyComponentWithUppy = enhance(MyComponent)

Or we could wait for React Hooks to be out of alpha, because I think it'll support the exact lifecycle that we need.

@goto-bus-stop goto-bus-stop merged commit 097c6ef into master Jan 28, 2019
@goto-bus-stop goto-bus-stop deleted the feature/react-uppy-change branch January 28, 2019 09:58
@oyeanuj
Copy link

oyeanuj commented Apr 2, 2020

@goto-bus-stop +1 for react-hooks support here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants