Bug Tracker

Ticket #8921 (closed enhancement: fixed)

Opened 4 years ago

Last modified 3 years ago

jQuery private data should stay private

Reported by: gnarf Owned by: gnarf
Priority: blocker Milestone: 1.7
Component: data Version: git
Keywords: 1.7-discuss Cc:
Blocking: Blocked by:

Description (last modified by ajpiano) (diff)

It seems that the "private" data is ALWAYS stored on the .data(jQuery.expando) - For "objects" where the deletion of the object should also delete its caches this makes some sense.

In the realm of nodes however, I think we should store these "private" members in a separate (private) cache so that they don't pollute the object returned by $.fn.data()

Change History

comment:1 Changed 4 years ago by gnarf

  • Owner set to gnarf
  • Status changed from new to assigned
  • Component changed from unfiled to data

comment:2 Changed 3 years ago by john

  • Keywords 1.7-discuss added

Nominating ticket for 1.7 discussion.

comment:3 Changed 3 years ago by rwaldron

  • Description modified (diff)

+0, Duplicates #8792

comment:4 Changed 3 years ago by jaubourg

+1, I know the code would probably grow (I'm not so sure but heh) but I think it makes a lot of sense

comment:5 Changed 3 years ago by timmywil

-1, use case where this is a problem?

comment:6 Changed 3 years ago by gnarf

  • Description modified (diff)

The reason I ran into the issue in the first place was that I changed the .data() behavior to store a flag in private data stopping it from scanning all the data- attribs more than once. Then a bunch of unit tests broke becuase it was using the jQuery.expando key on the .data() -- Basically, anytime you wanted to for ... in the .data() you're going to need to "ignore" the "private" data....

comment:7 Changed 3 years ago by dmethvin

+1, I was thinking the same, worth considering with the "attach data to DOM elements" rewrite if it can be done

comment:8 Changed 3 years ago by timmywil

  • Priority changed from undecided to high

comment:9 Changed 3 years ago by john

  • Description modified (diff)

+0, Don't really have strong feelings on this.

comment:10 Changed 3 years ago by ajpiano

  • Description modified (diff)

+1, Seems to be part of fixing #8909

comment:11 Changed 3 years ago by dmethvin

  • Priority changed from high to blocker
  • Milestone changed from 1.next to 1.7

comment:12 Changed 3 years ago by gnarf

  • Summary changed from jQuery private data should to jQuery private data should stay private

comment:13 Changed 3 years ago by gnarf

 https://github.com/jquery/jquery/pull/499

Pull currently closed while I look at another option.

comment:14 Changed 3 years ago by gnarf

I think  https://github.com/jquery/jquery/pull/500 turned out better than 499...

It changes it around so that instead of the "public data cache" storing the "private data" there is a "private data cache" which stores "public data"

comment:15 Changed 3 years ago by Corey Frang

  • Status changed from assigned to closed
  • Resolution set to fixed

Landing pull request 500. 1.7 - "public data" stored as a key on "internal data" - Fixes #8921.

More Details:

Note: See TracTickets for help on using tickets.