Could we make it less painful to inline ReactElement objects?

[ariabuckles]

Hi!

@spicyj said I should open an issue here. Disclaimer: I'm trying to do sort of sketchy things, and I know this isn't supported, but I think it would be nice.

I'm writing a library that I want to support react output optionally. To do this, right now I'm trying to inline all my ReactElements, such as

    return {
        type: 'hr',
        key: state.key,
        _isReactElement: true
    };

instead of

    React.createElement('hr', {
        key: state.key
    })

This is almost completely functional, except for the _store validation in react dev mode. This is because right now these literals don't have a _store key with .validated or .originalProps, which are assumed by the validators in React DEV. I can work around this, but to do so without duplicating props will have to write a wrapper function to create the element, which is a little unideal and slower (and adds unnecessary keys in non-dev mode).

Would it be reasonable to short circuit some of these checks so that they only happen if _store is actually present on the ReactElement? If so, I'd be happy to submit a pull request (no rush for react 0.13 though).

Curious as to your thoughts. Thanks!
Aria

[sophiebits]

(To be clear, this would be about changing checks like if (element._store.validated || element.key != null) in ReactElementValidator to not assume that element._store exists.)

@sebmarkbage This one's for you. :)

[syranide]

#3228 sounds very much related (but again, lack of warnings is an issue)

[sebmarkbage]

Note that createElement is responsible for both key AND propTypes warnings as of 0.13.

@ariabuckles The way I've been thinking about this problem is that this should definitely be possible to inline objects in your code. #3228 would only enable this as a production mode compiler feature. However, it should ONLY be ok in the case that you have some other way of providing valid warnings.

I don't think that simply opt-ing out of these warnings completely is a viable strategy. You might think that you won't mess up propTypes or keys as you're manually inlining these. Even if you don't, you might be accepting children or props from the outside that does need to be validated.

You might think that these warnings are coddling developers and that we should just allow you to opt out. However, this undermines the stability of shared components in a larger community - which ultimately will cause people to blame React.

One idea was to use a static type system like Flow. If you use a static type system, your files can be statically verified, and therefore you shouldn't need to validate them dynamically, even in development mode. I'm not sure how to accommodate this and transfer the verification flag. Perhaps using a special transform which adds the _store.

Can you explain a bit more about your use case (even if it is sketchy)? It might help us come up with ideas for how to solve your special case without losing warnings for the common case.

[ariabuckles]

Hi! Thanks for your response, @sebmarkbage.

I'm making https://github.com/khan/simple-markdown render to either react or html, and am figuring out how best to let html users not be messed up by a require("react") statement. There are a couple approaches I can take there, and not all of them require inlining elements, but inlining elements is promising because it completely removes the dependency on actually having React referenced by simple-markdown. Like you mention, it also means you can avoid the function call overhead in production, which given how many DOM nodes markdown often outputs, could be a win.

I'm not trying to avoid the warnings. I like react's warnings, and at KA we currently have most of them turned into alerts :). I'm mostly trying to avoid the dependency on React. I could solve this by dependency injecting it, which works but provides a slightly worse API for clients, and in this specific case I don't need refs and don't inject any children.

Unfortunately I am currently avoiding the warnings in my implementation, but (a) I would prefer not to in dev, although doing so is trickier, and (b) this specific case is a bunch of react elements I've hardcoded myself that don't have refs or external children, so I'm not too concerned about those warnings. key warnings would be nice, but at the same time the keys i'm using are pretty much the worst possible keys, and keyless elements wouldn't actually perform any worse.

Right now I've got a function for adding the _store prop to cheat validation ( https://github.com/Khan/simple-markdown/blob/react/simple-markdown.js#L212 ), but ideally could make it so that this function does validation in dev mode and skips it in production.

I'd probably not have said anything ordinarily, but Ben said I should.

[sebmarkbage]

It is definitely a goal to decouple the ReactElements from React itself. The idea is that a transpiler can have its own React.createElement implementation so that you don't need to have React in scope to generate these elements. Any such implementation would need to have both DEV and production mode I guess.

Another possibility is that we could start using WeakMap for _store as the element gets passed through.

[STRML]

This _store check in ReactElementValidator is causing exceptions when using babel's new inlineElements optimisation.

[zpao]

@STRML IIRC that is only seen when using the inline elements transform and the dev build of React. cc @spicyj

[STRML]

Yeah, this is in 0.14.0-rc1.

[sophiebits]

I had meant to fix that check but I guess I missed it. But yes, you should use the prod build of React when using the inlined elements.