Private name mappings (Imports)

jkrems commented 4 years ago

The proposal currently lists a secondary imports field. This hasn't been implemented anywhere afaik. We should decide what the solution for that use case is.

jkrems commented 4 years ago

The suggestions I'm aware of are:

Don't do anything. Packages that want to remap individual files can use self-reference and public entries in their exports fields.
Implement a new field (imports), roughly with the spec in the current README.
Introduce a new sigil in exports that can only be used in self-reference.
Introduce a new condition that only applies in self-reference.

My personal preference is to build on top of self-reference and conditionals and use a new condition to express this.

Imports

"name": "pkg",
"imports": { "#fetch": "./lib/fetch.mjs" }

import '#fetch'; // only visible to code within the package boundary

Exports Sigil

"name": "pkg",
"exports": { "./#fetch": "./lib/fetch.mjs" }

import 'pkg/#fetch'; // only visible to code when using self-reference imports

Exports Condition

"name": "pkg",
"exports": { "./fetch": { "internal": "./lib/fetch.mjs" } }

import 'pkg/fetch'; // only visible to code when using self-reference imports

guybedford commented 4 years ago

If you look at the example posted in https://github.com/uuidjs/uuid/pull/462#issuecomment-637854269 the first thing one notices is that "exports" is already more verbose than the browser field (and obviously because it carries the ability to hold more information than just browser mappings).

If we use an internal condition name I would be concerned with the level of verbosity in that case, since the internal mapping would then need to additionally compose with the browser mapping and any other mappings, leading to quite deep nesting.

I think it would be beneficial to try and ship something here especially given the resistance @sokra indicated to wanting to implement private mappings on the public API.

I would like to suggest that a variation on the sigil to use a ./# syntax:

{
  "name": "uuid",
  "exports": {
    "./#md5": {
      "browser": "./dist/md5-browser.js",
      "default": "./dist/md5.js"
    },
    "./#rng": {
      "browser": "./dist/rng-browser.js",
      "default": "./dist/rng.js"
    },
    "./#sha1": {
      "browser": "./dist/sha1-browser.js",
      "default": "./dist/sha1.js
    }
  }
}

By retaining the leading ./ we make it clear these are still subpath mappings leaving room for other types of "internal bare specifier" mappings in future as we have so carefully reserved to date as well.

Usage would be via import 'pkg/#internal' so remains fully symmetric with standard exports behaviour.

Note: this variation of the feature is fully valid under current exports - you can ship it now and it will work in Node.js.

That is, the # doesn't change anything else semantically apart from just the private restriction for non internal importers!

jkrems commented 4 years ago

Updated the sigil bit to include the ./. The exports sigil would definitely be my 2nd choice and those specifiers are "ugly" enough that they're pretty obviously not an official (public) API.

guybedford commented 4 years ago

The nice thing is that private names are well-enough adopted now for the meaning to be obvious to JS users.

sokra commented 4 years ago

Using # is not a good idea in my opinion. They have a special meaning in URLs (which these requests are): fragments. We would probably run into problems when trying to translate the exports field into an importMap.

Note: I'm only referring to usage in imports like import 'pkg/#fetch'; or import '#fetch';. Usage in the exports field would be fine.

sokra commented 4 years ago

Something like that would also be possible:

"name": "pkg",
"exports": { "#/fetch": "./lib/fetch.mjs" }
"exports": { "#./fetch": "./lib/fetch.mjs" }

import 'pkg/fetch'; // only visible to code when using self-reference imports

But I dislike that as it's not visible from importing site that this is private. I also dislike using self reference import as you have to repeat the package name.

sokra commented 4 years ago

So to summarize my wishlist would be:

clearly visible in import that it's private
different name than exports as it's not the public api
not using #, ?, + or % as thats reserved in urls
support for conditions like exports
key in mappings object is equal to request in `import "..."
similar syntax as exports
not colliding with absolute paths
not colliding with possible npm package names
support directory mappings like exports

It's only a wishlist, I'm also fine if not everything works.

Here are some options that would fullfill that:

import "@/fetch"
import "_/fetch"
import "[fetch]" and other brackets <>, {}, ()
import "$/fetch"
import "$fetch"
import "*fetch"
~import "*/fetch"~ (could be confused with globs)
import ":fetch"
import "|fetch"
~import "~/fetch"~ (often used as reference to app directory)
~import "~fetch"~ (often used as reference to app directory)
import "^fetch"
import "^/fetch"

jkrems commented 4 years ago

They have a special meaning in URLs (which these requests are): fragments. We would probably run into problems when trying to translate the exports field into an importMap.

A clarification here: The bare specifiers in import maps aren't URLs and # should be valid in bare specifiers unless import maps chooses to disallow them in the future. So far the direction of import maps has been to not put any restrictions on bare specifiers.

The # would be an issue on the right side (the mapping to a URL) but on the left side I don't expect problems with import maps as things stand.

guybedford commented 4 years ago

Yes I can confirm that # works in Node.js and browsers today with the current import maps and exports field implementations. That actually in many ways makes it a preferable option since it means it is definitely not a URL and hence can only exist in the "virtual space" of the LHS side of the export map.

I also really like the backwards compatibility approach to implementation that came up here.

sokra commented 4 years ago

If that's the case that's great and I'll take back my critique on that. In this case I prefer the "imports" field.

coreyfarrell commented 4 years ago

My preference is for the "Exports Sigil" where exports uses LHS ./#fetch and code uses import 'pkg/#fetch'. My complaint with import '#fetch' is that it could create difficulty if using minimatch on specifiers since a string starting with # is treated as a comment. I agree with @guybedford about the internal condition, I think it would be best to avoid adding more nesting to the exports structure.

Can we specifically mention that deep private exports work? For example "./#/": "./" and import 'pkg/#/private/file.js'.

One detail this "Exports Sigil" works today but is not restricted to self-reference.

{
  "name": "pkg",
  "version": "0.1.0",
  "exports": {
    "./#/": "./",
    "./#package.json": "./package.json"
  }
}

This package.json allows the following from inside or outside pkg:

console.log('via "./#/" export', require('pkg/#/package.json'));
console.log('via "./#package.json" export', require('pkg/#package.json'));

This is a good and bad thing. Technically making "./#/": "./" a private export is breaking, but on the other hand it would allow use of internal self-reference from any versions of node.js with self-reference.

guybedford commented 4 years ago

I've put together a PR here in https://github.com/nodejs/node/pull/33780. Feedback welcome.

jkrems / proposal-pkg-exports