GoogleChromeLabs / confluence

Service and API for Web API Confluence: Metrics for web platform health
https://web-confluence.appspot.com
BSD 3-Clause "New" or "Revised" License
92 stars 36 forks source link

All Web IDL constants are missing #260

Open foolip opened 6 years ago

foolip commented 6 years ago

I was using confluence to research https://github.com/w3c/svgwg/issues/291 a bit today, to see if https://svgwg.org/svg2-draft/types.html#InterfaceSVGUnitTypes was implemented everywhere.

The interface object is there (everywhere), but the constants aren't listed. They are own properties of the interface object, and I do see SVG_UNIT_TYPE_UNKNOWN in the data collected in https://github.com/mdittmer/web-apis.

Other interfaces I checked to confirm missing constants: Event, Node. HTMLMediaElement.

Per https://github.com/GoogleChromeLabs/confluence/issues/202#issuecomment-332992863 this seems to not be an accident, but it still seems like something to fix.

mdittmer commented 6 years ago

This was by design after we observed that "actual" constants that (usually) look like SomeConstructor.SOME_CONSTANT were bloating API counts. I have implemented a CONST_CASE check at #296 and it is staged with fresh data here. The biggest surprise I saw there was Firefox's jump in browser-specific APIs.

mdittmer commented 6 years ago

@bzbarsky do you think the data at the const-case staging instance is more or less accurate than prod wrt Firefox APIs?

bzbarsky commented 6 years ago

I wish I could tell you. The staging instance shows mostly "..." for me various places without enough label text to actually say anything about anything.

bzbarsky commented 6 years ago

That said, if you are just adding constants, then I would expect to see 372 things added from the DOMVK* constants on KeyEvent and KeyboardEvent. Is that about what you're seeing?

bzbarsky commented 6 years ago

(Those are the constants from https://www.w3.org/TR/1999/WD-DOM-Level-2-19990923/events.html#Events-KeyEvent fwiw.)

mdittmer commented 6 years ago

The change is actually to include APIs that appear to be constants (i.e., have a "value" in descriptor, rather than getter/setter) but are not CONST_CASE. Prod exclude valued properties regardless of their name. So, the change should be purely additive, and should not introduce CONST_CASE properties.

On Fri, May 18, 2018, 3:57 PM Boris Zbarsky, notifications@github.com wrote:

(Those are the constants from https://www.w3.org/TR/1999/WD-DOM-Level-2-19990923/events.html#Events-KeyEvent fwiw.)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/GoogleChromeLabs/confluence/issues/260#issuecomment-390315854, or mute the thread https://github.com/notifications/unsubscribe-auth/ABsWSCVXCHqbWHxgFGqOLaASdbFGxwaJks5tzye4gaJpZM4Rz3oh .

bzbarsky commented 6 years ago

Well, https://const-case-data-dot-web-confluence.appspot.com/ is certainly showing CONST_CASE things.

mdittmer commented 6 years ago

Oops. I fixed the code without updating the data. Please try the staging URL now. Most (not all) CONST_CASEs are now filtered out, but other "value"d things are included.

Looks like some additional filtering is needed to drop non-APIs that accidentally shipped with browsers or bleed through from the data collection methods (e.g., interface names +toString+ and a)

foolip commented 6 years ago

@mdittmer, https://const-case-data-dot-web-confluence.appspot.com/ includes stuff that isn't in any of the 4 listed browsers, is that intentional? Also I'm not able to scroll with page up/down, that would be nice.

foolip commented 6 years ago

Another somewhat surprising thing is that searching for "HTMLMedia" doesn't find HTMLMediaElement. I know that's because the data isn't loaded yet, but that didn't stop me from trying.

foolip commented 6 years ago

HTMLMediaElement#HAVE_NOTHING now shows up, and I don't see HAVE_NOTHING anywhere else, so constants on interface objects (constructors) aren't being filtered out, that's good! I wonder if there are any things that are filtered out that aren't included on some other "object" instead?

bzbarsky commented 6 years ago

At this point I'm really not sure what the new thing is trying to measure. If it's trying to measure webidl constants, why is it excluding CONST_CASE stuff?

@mdittmer is there an existing issue on the new UI being a lot less usable than the old UI? Or do you need one filed?

foolip commented 6 years ago

@bzbarsky, the problem was that some things that weren't CONST_CASE were previously excluded, like in https://github.com/GoogleChromeLabs/confluence/issues/259. Including those is the most important thing. But IDL constants also show up both on the interface object and on instances, so there's a risk of double counting. @mdittmer, can you list which APIs are excluded by the new logic?

bzbarsky commented 6 years ago

But IDL constants also show up both on the interface object and on instances

Well, on the interface object and on the prototype. And yes, there is risk of double-counting.

I don't have a strong opinion about whether IDL constants should be included or not; I agree it's important to include things like static methods.

If you just want to exclude IDL constants, then excluding value properties which are non-writable is possibly a better approach than excluding based on name, imo. In Chrome, location.href is writable, of course.

foolip commented 6 years ago

Well, on the interface object and on the prototype

Argh, right you are. That just makes the double counting more certain, don't need to find an instance to have the problem.