Update types to use autoFocus instead of autofocus

[aaronjensen]

React uses autoFocus and that's what seems to work most reliably in preact as well, so I think this type may just be wrong.

[coveralls]

Coverage remained the same at 100.0% when pulling 08a2ace on aaronjensen:patch-1 into e5c7f1d on developit:master.

[developit]

both work, just autoFocus sets via an attribute whereas autofocus sets via a property. autofocus is probably ideal here though, since I think this is a bit odd:

<input autoFocus={false} />
// produces
<input autoFocus="false">

(I think)

[aaronjensen]

autofocus doesn't work in glamorous with preact-compat. We had to switch to using autoFocus.

<input autoFocus={false} />
// produces
<input autoFocus="false">

If that's true, I think that's broken. It should produce <input> w/o any autofocus attribute. Also, the html attribute is autofocus, not autoFocus as far as I can tell.

[developit]

You're styling an attribute (since properties are inaccessible from CSS), so you need to render an attribute. autoFocus is a case-mismatch from the DOM property, so it produces an attribute.

Perhaps it makes sense to include types for both? Technically autofocus is a Boolean whereas autoFocus is a String|Boolean (thanks to setAttribute's casting).

context: The autofocus attribute is case insensitive (most are) - the issue is more that autofocus is a match for a DOM reflected property. Preact doesn't include any special behaviors for these types of things because the definitions required would be prohibitively large and create a false API where there is currently only a mirroring of the DOM's API.

[aaronjensen]

React apparently doesn't actually render the autofocus property regardless of how you case it. They must handle it specially, so that's a difference between react and preact, I guess.

Would it make sense to patch glamorous to have it treat passed through properties in a case insensitive way?

/cc @kentcdodds

[kentcdodds]

Perhaps it makes sense to include types for both? Technically autofocus is a Boolean whereas autoFocus is a String|Boolean (thanks to setAttribute's casting).

I think this makes the most sense. They really are two different things aren't they?

Would it make sense to patch glamorous to have it treat passed through properties in a case insensitive way?

I'm uncertain I follow exactly what you're suggesting here.

[aaronjensen]

I'm uncertain I follow exactly what you're suggesting here

Sorry, I wasn't very clear. As far as I can tell, glamorous white lists attributes that are passed through to the React input element. autoFocus is one, but autofocus is not, making me wonder if the whitelist should be case insensitive. Does that make sense?

[kentcdodds]

Yes, that makes sense. I think that this is actually a bug in glamorous' dependency: react-html-attributes Could you file a PR there? 👍

[aaronjensen]

Sure, but what would it be? It looks like it's missing autoFocus entirely, so I could add that, but then would glamorous pass through autofocus? i.e. is it already case-insensitive?

[kentcdodds]

Actually, glamorous uses both react-html-attributes its own custom list of React props (which includes autoFocus). So I think that we'd just need to add autofocus to the right element(s) in react-html-attributes and that should take care of the issue.

[aaronjensen]

@kentcdodds I've submitted that, but I'm not sure it's right. React doesn't officially support autofocus, they support autoFocus as far as I can tell, so that would be the appropriate one to submit to react-html-elements, which would not help me here.

I'm not sure that I totally understand preact's stance on these attributes or why React capitalizes some of them and preact doesn't.

[kentcdodds]

Yeah, you're right. I think that it should be autoFocus... 🤔

[aaronjensen]

Ok, so should we add autofocus to glamorous's own list? Or should I just add it to the types here?

@developit is there any planned support for React's brand of autoFocus?

[developit]

No support planned, no - Preact will never customize or alter the behavior of DOM attributes/properties.

[coveralls]

Coverage remained the same at 100.0% when pulling 7866089 on aaronjensen:patch-1 into 4a9aefc on developit:master.

[aaronjensen]

Preact will never customize or alter the behavior of DOM attributes/properties.

Got it, thanks. So what do you think I should do here? I can just add autoFocus as a boolean (I'm not sure it's necessary to allow a string, is it?) and keep autofocus as it is.

I guess I was confused because autoPlay is camel case though its property is autoplay. Is that set via attribute too? Is there a reason to have both for everything?

Btw, is the attribute vs property behavior of preact documented somewhere?

[kentcdodds]

I wonder whether you could use forwardProps to fix this for your use case?

That said, we really should make glamorous work out of the box with preact. Feel free to file an issue and we can look into a real solution.

[marvinhagemeister]

@aaronjensen Seems like this issue has been resolved in the preact adapter from glamorous. Is this something you'd rather see in preact core itself or should this PR be closed?

[aaronjensen]

works for me

[aaronjensen]

This has reared its head again with emotion. A styled.div({}) with autofocus set does not work. If you set autoFocus, it works, but does not typecheck.

ref: emotion-js/emotion#734