Skip to content

Add uncallableAndIsHTMLDDA support#41

Merged
bterlson merged 3 commits intotc39:masterfrom
rwaldron:uncallableAndIsHTMLDDA
Nov 3, 2017
Merged

Add uncallableAndIsHTMLDDA support#41
bterlson merged 3 commits intotc39:masterfrom
rwaldron:uncallableAndIsHTMLDDA

Conversation

@rwaldron
Copy link
Copy Markdown
Contributor

To support this issue created in Test262, @jswalden proposed the addition of this host api extension.

@bakkot
Copy link
Copy Markdown
Member

bakkot commented Oct 17, 2017

I think somewhere I've seen a builtin function for d8 that acts similarly, but it might require the % internal syntax and so couldn't be used in this manner.

It's %GetUndetectable(), and yes, it requires --allow-natives-syntax.

@rwaldron
Copy link
Copy Markdown
Contributor Author

@bterlson the appveyor failure is expected (I just need to finish the work to make this fully tested on windows)

@bterlson
Copy link
Copy Markdown
Member

@rwaldron Confirmed ch.exe does not have an API to get the ULEO :(

I like everything about this PR but the API name. How about something like "getHTMLDDAObject" or something?

@jswalden
Copy link
Copy Markdown

The API name is as it is because the purpose of the function is to expose 1) an object with an [[IsHTMLDDA]] internal slot (if possible), but also 2) an object that when called in certain manners will throw a TypeError. This is so because the three tests recently landed that use this, invoke ECMA-262 functionality that will call such an object -- and the behavior of that object when called in those particular manners must be specified. "getHTMLDDAObject" as name gets across point 1, but it says nothing about point 2. The name is an awkward word salad, but it does get across the behaviors it's required to have.

@bterlson
Copy link
Copy Markdown
Member

Alright, seems fine. Ready when you are @rwaldron.

@bakkot
Copy link
Copy Markdown
Member

bakkot commented Oct 19, 2017

@bterlson I think the test262 API may be wrong; see tc39/test262#1304. I would suggest holding off on this pending resolution there.

It may be that I'm just confused - I'm confused about something somewhere - but if I'm reading things right this API shouldn't be called uncallable.

@rwaldron
Copy link
Copy Markdown
Contributor Author

per @bakkot's status update:

I think the test262 API may be wrong; see tc39/test262#1304. I would suggest holding off on this pending resolution there.

/me nods.

I've updated to fakeDocumentAll, but this should still wait until that PR has landed

@rwaldron rwaldron force-pushed the uncallableAndIsHTMLDDA branch from 2da3680 to ae6cab0 Compare October 19, 2017 17:52
@jswalden
Copy link
Copy Markdown

FYI, with the final changes to that API to make it $262.IsHTMLDDA as an object-valued property, that property is only implementable in browser hosts -- or in shells that have implemented the exact contract required here. In new-enough SpiderMonkey shells (i.e. after the patch for https://bugzilla.mozilla.org/show_bug.cgi?id=1410194 I landed today), createIsHTMLDDA() will give you back an object suitable as the value of that property. Other shells probably won't be able to support this til they make similar tweaks.

@bterlson
Copy link
Copy Markdown
Member

bterlson commented Nov 3, 2017

Sorry for the delay (ended up away from office most of this week). I think this is fine (name and all) upon reflection.

@bterlson bterlson merged commit 1b735a4 into tc39:master Nov 3, 2017
@jswalden
Copy link
Copy Markdown

jswalden commented Nov 3, 2017

Erm, test262 changed to have the name be $.IsHTMLDDA as a document.all-like object. See tc39/test262@c05138b for details. So having the name be fakeDocumentAll as in 1b735a4 seems pretty wrong. Or is there going to be the rename-followup very quickly after this?

@bterlson
Copy link
Copy Markdown
Member

bterlson commented Nov 3, 2017

@jswalden I don't like prefixing a non-Boolean with is :( My thinking was in test262-harness I just need to shim this (it's not necessarily a goal to support the entire test262 API in this project). Thoughts?

@bterlson
Copy link
Copy Markdown
Member

bterlson commented Nov 3, 2017

Would just "HTMLDDA" be better for anyone?

@jswalden
Copy link
Copy Markdown

jswalden commented Nov 3, 2017

I'm not horribly happy about an "is" thing that's an object and not a function predicate, but I got over it enough to run with it. :-) HTMLDDA is fine by me as well, with caveat that it's just a bikeshed and all and I really don't care except very mild hesitance to change something that I thought I'd dealt with and made consistent everywhere.

But maybe my understanding is incorrect -- I though this really was trying to recreate the $262 API across all implementations, and so whatever name was chosen would be exactly dictated by that.

@bterlson
Copy link
Copy Markdown
Member

bterlson commented Nov 3, 2017

This project's goal is to create a useful utility and library for all sorts of cross-host-compatible scenarios. Test262 is an obvious place where this is important, but there are others too! Including the equally important scenario (IMO) of just experimenting or exploring compat differences where having nicely named APIs is important from a usability perspective.

I'll go with your mild hesitance and change it to IsHTMLDDA. This will not be a popularly used API, and I'm also happy to make breaking changes in major versions if we want to change the name later ;)

@bterlson
Copy link
Copy Markdown
Member

bterlson commented Nov 3, 2017

see 5297b51.

weswigham added a commit to weswigham/eshost that referenced this pull request Jan 28, 2020
To allow the harness to be used with very old versions of node that predate es6 support (like `node 0.10`). It used to be like this before tc39#41 seems like.
rwaldron pushed a commit that referenced this pull request Jan 29, 2020
To allow the harness to be used with very old versions of node that predate es6 support (like `node 0.10`). It used to be like this before #41 seems like.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants