feat!: add did mailto package, replacing createDidMailtoFromEmail

[gobengo]

Motivation:

• publish @web3-storage/did-mailto package #682

Breaking Changes

• remove createDidMailtoFromEmail export from @web3-storage/access/agent 93e29c6#diff-69a4efe733b2d7920dc103a0370eb1a285403e39c41e1b5e9a0718ea66b5a32fL33
  • no dependencies under our org: https://github.com/search?q=org%3Aweb3-storage%20createDidMailtoFromEmail&type=code
  • Motivation: encouragement from @Gozala and @travis in review

[Gozala]

Suggested some suggestions, but ok to land as is as well

[Gozala]

Nit: Could we get rid of this module ? Maybe we could just add a function that does this into the new package ?

[gobengo]

Originally I didn't want to make a breaking change, and this function was exported from the package, but since both you and travis suggested it, and I verified we have no other repos depending on it, I went ahead and removed and marked this a breaking change.
93e29c6#diff-7136e328e23c0e6a0f91abc8e3007589dc0774d34a249b2001f9126d7d364029

[Gozala]

Empty domain does not seems valid and should be an error perhaps worth considering checking it also has . in it.

[Gozala]

Nit: I personally would find it easier to follow this if it was something like:

const offset = email.lastIndexOf('@')
if (offset < 0 || email.length - offset <= 1) {
   throw
} else {
  const local = emal.slice(0, offset)
  const domain = email.slice(offset + 1)
}

[Gozala]

Can we test that following fail email@ and [email protected]

[gobengo]

Both of those strings would match the EmailAddress type I have in here.
I'm willing to make it more picky to disallow those two cases, but let's do it in another PR. I looked into doing more strict email address parsing, and to do it fully feels like more work than is really useful. (and if you need that, they exist).

[travis]

what's this email() function for? it seems like it pulls the email address apart just to put it back together again.

If we get rid of this then we can also get rid of https://github.com/web3-storage/w3up/pull/722/files#diff-7136e328e23c0e6a0f91abc8e3007589dc0774d34a249b2001f9126d7d364029 which would be great.

At the very least, could you add a comment here explaining why this exists?

[gobengo]

it accepts a string and returns an EmailAddress (which is more specific than a string)

it seems like it pulls the email address apart just to put it back together again.

It also throws an error if what you passed it is definitely not an EmailAddress

The motivation was because in @web3-storage/access/agent, sometimes email is string and sometimes its ${string}@${string}. In this commit you can see how when removing createDidMailtoFromEmail, one spot needed to parse string -> ${string}@${string} using this function, and another did not.

[gobengo]

added a comment. 3b1ff2b#diff-6caa234bd554caeddcc0602f2a8ffc66e596debc4b1372f0fdfdb067b3d9f3b4R30

[travis]

ah I see. it feels kind of weird to be doing runtime work (ie, pulling the string apart and then reassembling it into the same string) for what should be compile-time typechecking (ie, ensuring that strings that are emails have the EmailAddress type during compilation)

I could see this being valuable if we thought we might change the EmailAddress type in the future to be more complicated without changing all these APIs - is that the kind of flexibility you're hoping to buy with this?

If that's not an explicit goal, I wonder if it might be better to have fromEmail take a string | EmailAddress and throw if the string isn't an actual EmailAddress?

I guess one nice thing about factoring things this way is that you make that string -> EmailAddress conversion optional, only needed by folks who haven't already checked and case the raw string. This line of thinking makes me think that maybe the name is the problem here - email() just feels like a puzzling thing to have to call on my string before I can use it with this API - maybe stringToEmailAddress or ensureEmailAddress or something would read a bit better?

I'm also fine with just leaving as-is, but I may propose a name change in the future :)

[gobengo]

I could see this being valuable if we thought we might change the EmailAddress type in the future to be more complicated without changing all these APIs - is that the kind of flexibility you're hoping to buy with this?

yeah

I guess one nice thing about factoring things this way is that you make that string -> EmailAddress conversion optional, only needed by folks who haven't already checked and case the raw string.

👍

but I may propose a name change in the future :)

That's okay. afaic there can be many names, and it's unlikely everyone will be happy with any one name.
https://en.wikipedia.org/wiki/Law_of_triviality