chore: move Uploader into react-uploader package #137
Conversation
travis
force-pushed
the
chore/w3ui-ui-move-uploader
branch
from
ef284ed to
d48675e
Compare
|
My only point of friction on this is somebody who previously installed the package as-is later wanting to improve upon it with a few customizations will need to rip out previous work and re-install as opposed to ad-hoc changes. Other than that I don't have a strong opinion on separating the packages this way. |
|
Actually after a little more thought I do agree that it makes sense to split out the package that is "styled" into a *-ui. It does help imply what you are importing and lets the functional components stand by themselves in a cleaner way. |
I think it makes more sense to include these "headless UI" style components in the @w3ui/react-* packages rather than bundling them together in the react-ui package. It feels more intuitive given the naming - if we do want to have the "no UI" style components in a separate package I'd advocate for callling that package `react-uploader-provider` or `react-uploader-api` or something, but given how much code this is and the fact that tree shaking is pretty good, it seems ok to me to bundle them together. the react-ui package will be reserved for higher level "customizable UI" style components and can now have opinions about the right way to do customization at the top level rather than having two different flavors of customizability in a single package.
travis
force-pushed
the
chore/w3ui-ui-move-uploader
branch
from
d48675e to
038ad22
Compare
This is a good point though! Making a mental note to make sure to document this explicitly to help folks understand how to make this transition smoothly - I do think that this is a cleaner way to separate things, but "moving down the abstraction stack" here is definitely a little more complicated - developers will need to a) find where the components that underlie the current component they're working on live and b) import and use them - I think in both cases looking at the source code will be super helpful, so we should make sure to have links to the GitHub source in documentation of the "customizable UI" components. |
* make it work more like the UploaderContext in the providers/Uploader.tsx * rename it from UploaderContext to avoid a name collision * be more careful with memoizing the provider value to avoid unecessary re-renders from inline object literal creation (TODO: need to do this in the provider too)
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a6f31fe:
|
|
adding @jchris - this is the PR I mentioned this morning |
|
I'm going to merge this since it's just a merge into my w3ui-ui feature branch, but still happy to revisit this decision |
I think it makes more sense to include these "headless UI" style components in the @w3ui/react-* packages rather than bundling them together in the react-ui package. It feels more intuitive given the naming - if we do want to have the "no UI" style components in a separate package I'd advocate for callling that package
react-uploader-providerorreact-uploader-apior something, but given how much code this is and the fact that tree shaking is pretty good, it seems ok to me to bundle them together.the react-ui package will be reserved for higher level "customizable UI" style components and can now have opinions about the right way to do customization at the top level rather than having two different flavors of customizability in a single package.
@alanshaw @cmunns curious what you think about this - happy to move the uploader back if we decide that's a better structure