Closed joshwcomeau closed 6 years ago
i can understand and respect the simplicity argument. it requires a different level of knowledge of react (and react-flip-move) to pass it correctly. that is - ultimately - what this decision hinges on imo. Ease of use vs semantical correctness. Personally i think we should aim to get rid of all DOM-references though. Its whats holding flip-move back more than anything else :P
but maybe we can use any falsy value
i dont think so. Unless you want wrapperless mode to replace default mode (with no typeName
being provided). Which might be cool, but could break a lot of implementations.
Personally i think we should aim to get rid of all DOM-references though. Its whats holding flip-move back more than anything else :P
Can you elaborate on this? In what ways is it limiting us, what are the problems you see with relying on it?
(also, I assume you mean findDOMNode
specifically? Or are you including other DOM interaction as well?)
i dont think so. Unless you want wrapperless mode to replace default mode (with no typeName being provided). Which might be cool, but could break a lot of implementations.
Well, there'd still be the defaultProp
value for typeName
. I just mean accepting null
and false
(or typeName={undefined}
, I suppose? I believe there is a difference between supplying undefined
explicitly, and just not supplying a value... but yeah this is a tangent). Because yeah I definitely don't think we should change the default behaviour (maybe in a future major version, if a couple versions pass with wrapperless mode working well)
I believe there is a difference between supplying undefined explicitly, and just not supplying a value
In terms of applying defaultProps
there's none
sure i can elaborate, but its not really actionable and only relates to this issue from a 10k feet view.
i was talking about all interactions with the DOM, not just findDOMNode
. Its the limiting factor in terms of performance, and we're seeing a lot of issues arising from the manual synchronization between the two.
It would be really nice if we could calculate animation state for each element without any DOM references at all, and then apply those styles with some CSS-in-JS solution. That's the direction i'd like to see this library go in at least
In terms of applying defaultProps there's none
Cool, yeah, good to know!
It would be really nice if we could calculate animation state for each element without any DOM references at all, and then apply those styles with some CSS-in-JS solution. That's the direction i'd like to see this library go in at least
Interesting! I'm not sure that's possible, though, unless the user enters information about size and position themselves. The bottleneck is the DOM because we need to figure out where to move stuff to, so unless the user provides that info explicitly, we need to query the DOM, right? And Flip Move's core philosophy is zero-configuration, drop-it-in-and-go, so I'd be wary about asking the user for any required props like that.
I think it'd be worthwhile to create a separate library that sacrifices ease-of-use for performance and reliability. That way, Flip Move can be the drop-in, works-well-enough-for-most-usecases library you try first, and if it doesn't work, we recommend they use the other library. If anyone wants to build it, I'd be happy to link to it, and possibly contribute :)
But yeah, so I think in the meantime I'm going to ditch the anchor
and the associated warning; I agree with everything you said, @tobilen, but I think the DOM-free solution ought to be a separate thing.
Sorry for the delay; between travelling and the flu, I haven't had a ton of time/energy. Will try and get a PR up today though!
I'm working on some tweaks for the next version, which features the "wrapperless" mode @tobilen worked on.
I'd like to propose that we ditch
anchor
and make thefindDOMNode
behaviour the default.While this sounds radical, here's my rationale:
findDOMNode
is an escape hatch that won't be deprecated anytime soon. It's heavily discouraged for in-app use, but I think similar tocontext
, it's fine for libraries to use when we know what we're doing. We're already using it to resolve composite components passed in as children.Flip Move's value prop is "effortless, magical animation for free". I feel like we're exposing the user to our internal complexity by having them supply an
anchor
; even just based on the issue in the comments, I suspect many folks will be confused / struggle with thisAFAICT,
findDOMNode
isn't that computationally expensive. I did a quick test with an admittedly small DOM and it figured it out in <1ms, in dev mode. We're already calling findDOMNode once per child, on every render (when composite components are supplied as children), and so I'm not very concerned about doing it a single time for a single element.Simpler logic means it'll be more maintainable. I like the idea of reducing complexity when possible, and I can picture lots of edge-case quirks (what if the user provides an
anchor
from somewhere else on the page? What if thatanchor
is deleted outside of React? etc)So yeah, I think we should just advocate for providing
null
totypeName
(I prefernull
tofalse
because it feels more semantic - there is NO typeName, rather than a false typeName - but maybe we can use any falsy value?) if the user doesn't want a wrapper.Anything I'm missing? cc @tobilen @Hypnosphi