Fix return type of CredentialsContainer.store

[lukewarlow]

Fixes #214 #98

---

Preview | Diff

[rschristian]

Would be really nice to get this merged! I was a bit miffed with my tests until I thought to come here and see if anyone had the same issue.

[lukewarlow]

@npm1 are you able to take a look at this? Or direct the correct person to review it?

[johannhof]

@nsatragno ^

[npm1]

FWIW the IDL in Chromium says Promise<Credential> but the promise is being resolved with nothing.

[lukewarlow]

FWIW the IDL in Chromium says Promise<Credential> but the promise is being resolved with nothing.

Yeah that along with the typescript types being wrong is what led me here.

[nsatragno]

Oh I am so sorry I did not take a look earlier, for some reason this PR never sent me a notification. This issue is related to #98 and we might be able to close both with the same PR.

I agree we should update the spec to match implementation and have the promise return undefined. Could you also update the references to the return value?

Step 7.1 of Store a Credential should say "Resolve p with undefined" instead. There is no point resolving with r if we'll always return undefined.

I think we can also do away with Return undefined on PasswordCredential's [[store]] and FederatedCredential's [[store]].

Another interesting bit is that the default implementation for [[store]] says to return undefined, but in practice we throw instead. Throwing IMO is better as a signal that the developer is doing something that is not supported, I filed #218 to track this.

(It looks like the build is broken again too -- don't worry about spurious errors, for now we can force-push the PR and fix them later)

[lukewarlow]

No worries! I'll make those changes tomorrow.

[lukewarlow]

@nsatragno Those changes should be done now

[w3cbot]

nsatragno marked as non substantive for IPR from ash-nazg.

[nsatragno]

Approved, thanks!