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

react: Allow changing instance in uppy prop #1247

Merged
merged 2 commits into from
Jan 28, 2019

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