fission-codes / keystore-idb

In-browser key management with IndexedDB and the Web Crypto API
Apache License 2.0
56 stars 8 forks source link

Replace rollup with esbuild #53

Closed icidasset closed 3 years ago

icidasset commented 3 years ago

Pretty much copied from webnative, except for a few things. An important one being:

{
  "exports": {
    "./*": "./lib/*", // instead of "./lib/*.js"
  }
}

When I used *.js in webnative, it would try to use double extensions (ie. .js.js) when doing imports. I thought I'd add "./*.js": "./lib/*.js" to the exports map, but no luck, so I did the above. I guess this removes the option to do extensionless imports, but weren't bundlers moving away from that anyway?

matheus23 commented 3 years ago

I guess this removes the option to do extensionless imports, but weren't bundlers moving away from that anyway?

It's... complicated. :D

And it's good that you bring this up. There's two ways this could go.

The issue is with typescript supporting extensionless internal imports. So let's say we're in webnative source code, and we're importing ./fs/filesystem. This is only supported by typescript and some bundlers (e.g. esbuild), but when typescript compiles this to javascript, it'll result in the line import [...] from "./fs/filesystem". Node wouldn't be able to understand that import, as there's no extensionless filesystem file there. That's why you should use ./fs/filesystem.js for relative, internal imports.

But when there's a package boundary between the file and what's imported, then export maps define the behavior and so far it seems like most bundlers do the same thing there.

And it seems somewhat nicer to be able to import "keystore-idb/rsa/keystore" instead of import "keystore-idb/rsa/keystore.js", that's why I was going with that.

But actually that notion should be challenged. Maybe we want users to always include extensions?

Also, there might be a way to have users be able to both import with and without extensions using export maps. I.e. would this work?

{
  "exports": {
    "./*.js": "./lib/*",
    "./*": "./lib/*.js"
  }
}
icidasset commented 3 years ago

Sigh, I don't know 😄 We'll let the ecosystem decide I guess. If all bundlers support the extension-less imports, only then we'll use that.

It doesn't seem to match on "./*.js", only "./*" (even when placed before "./*")

icidasset commented 3 years ago

Totally forgot that eslint also has auto fix stuff 😫