Skip to content

Data: update both hyphenated and camel case keys #1515

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

Closed
wants to merge 1 commit into from

Conversation

rwaldron
Copy link
Member

Fixes #14799

@gibson042
Copy link
Member

Given that we're talking about breaking changes anyway, I think any commit is premature before we settle on a desired API. The possibilities are numerous:

1.x Backcompatiphilia

  • attribute reader camelCases but never overwrites
  • setter( key, value ) camelCases (and therefore ignores hyphenated keys from setter( obj ))
    • (dropping 2.x behavior of writing to hyphenated keys after overwriting corresponding camelCased keys)
  • setter( obj ) shallow jQuery.extends (and therefore does not camelCase)
  • getter( key ) uses camelCase for fallback
  • getter() returns only camelCase keys except for those from setter( obj )
    • (dropping 2.x behavior of also returning hyphenated keys after setter( hyphenated, value ) overwrites data)
  • weird cases follow setter( objWithHyphenatedKey )
    • signature-dependent key translation: "uh-oh" in obj.data({ "uh-oh": "foo" }).data() ); !( "uh-oh" in obj.data( "uh-oh", "foo" ).data() )
    • hyphenated/camelCase mismatch: obj.data({ "uh-oh": "foo" }).data( "uh-oh", "bar" ).data( "uh-oh" ) === "foo"; obj.data({ "uh-oh": "foo" }).data( "uh-oh", "bar" ).data( "uhOh" ) === "bar"

Embrace HTML5

  • attribute reader camelCases but never overwrites
  • setter( key, value ) camelCases
  • setter( obj ) camelCases all properties (and therefore can self-overwrite)
  • getter( key ) camelCases before lookup
  • getter() returns only camelCase keys
  • weird cases follow setter( objWithHyphenatedKey )
    • key translation: !( "uh-oh" in obj.data({ "uh-oh": "foo" }).data() ); !( "uh-oh" in obj.data( "uh-oh", "foo" ).data() )

Garbage In, Garbage Out

  • attribute reader camelCases but never overwrites
  • setter( key, value ) does not camelCase
  • setter( obj ) shallow jQuery.extends (and therefore does not camelCase)
  • getter( key ) uses camelCase for fallback (or doesn't; who cares?)
  • getter() returns whatever mix was used for setting
  • weird cases abound, but only affect inconsistent-input scenarios

Smorgasbord

  • attribute reader tries to set both hyphenated and camelCase keys, but never overwrites
  • setter( key, value ) sets both unmodified and camelCase keys
  • setter( obj ) sets both unmodified and camelCase keys
  • getter( key ) uses unmodified input
  • getter() returns a identical data under a pair of keys for each input
  • weird cases: I dunno, all of them?

Super-Smorgasbord

  • like "Smorgasbord", but we also reverse camelCase input keys

I'm partial to "Embrace HTML5" and "Garbage In, Garbage Out".

Also, it's worth remembering that all of them technically allow tricky business via manipulating the (otherwise purely internal) object returned from a no-argument getter (e.g., obj.data()[ "I-can-hyphenate-all-I-want-2!" ] = true).

@dmethvin
Copy link
Member

@gibson042 thanks for laying out all the options. I like GIGO myself but will think about it some more.

@dmethvin
Copy link
Member

Actually I created a spreadsheet to see the carnage at a glance. I'm thinking the Embrace HTML5 option may cause less breakage since we have said we camelized internally, but camelizing the keys we merge bugs me. Ask for edit access if I've screwed something up, which seems likely.

@dmethvin
Copy link
Member

Per our discussion in the core meeting we decided to leave this behavior for the .1 releases but change it to "Embrase HTML5" in 2.2, making behavior more consistent with 1.12. We don't want to have both forms stored in the data cache.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants