How to blacklist specific directories?

[jaydenseric]

A common requirement is to expose an entire directory to the public, but not a sub-directory of internal helpers.

For example, with:

{
  "files": [
    "lib"
  ],
  "main": "lib",
  "exports": {
    ".": "./lib/index.js",
    "./lib/": "./lib/"
  }
}

How can you make ./lib/helpers/ private?

It's possible to move the helpers directory out of the lib directory to be at the same level, but sometimes this is not desirable. For example, graphql-react has helper files scoped under universal, server and test directories so that they can have seperate .babelrc.js files for transpilation per environment:

https://github.com/jaydenseric/graphql-react/tree/v10.0.0/src

Ideally the helper files could be gathered into helpers directories, and blacklisted in place. Moving the helpers out from under the scope of the .babelrc files would be a hassle, it would mean duplicating the Babel config files, so 3 becomes 6 with repetition.

Alternatively, the structure could be changed to graphql-react/universal/lib/foo.mjs and graphql-react/universal/helpers/bar.mjs, but inserting the extra /lib into the paths is a breaking change to the existing paths and looks ugly for consumers to write.

[jaydenseric]

@ljharb not sure how that is any different to putting additional exports rules in the top level package.json?

[ljharb]

True, it's basically the same.

[jaydenseric]

Ok, here is one possible way to blacklist that seems to work:

{
  "name": "a",
  "main": "lib",
  "exports": {
    ".": "./lib/index.js",
    "./lib/": "./lib/",
    "./lib/helpers/": "./this-dir-doesnt-exist/"
  }
}

For ./this-dir-doesnt-exist/, it doesn't seem to matter if the directory truly doesn't exist, or if you put an empty directory there. The MODULE_NOT_FOUND error if you try to require('a/lib/helpers/index.js') is the same.

Is there a problem with doing this, will people understand that this fake path is on purpose, and not a mistake?

[ljharb]

Instead of a fake path, what if you do null or []?

[jaydenseric]

Good ideas, here's the results.

1. null:

   internal/modules/cjs/loader.js:631
     throw new ERR_INVALID_PACKAGE_TARGET(
     ^
   
   Error [ERR_INVALID_PACKAGE_TARGET]: Invalid "exports" target null defined for './lib/helpers/' in the package config /Users/jaydenseric/Desktop/package-exports-test/node_modules/a/package.json
       at resolveExportsTarget (internal/modules/cjs/loader.js:631:9)
       at applyExports (internal/modules/cjs/loader.js:519:14)
       at resolveExports (internal/modules/cjs/loader.js:541:12)
       at Function.Module._findPath (internal/modules/cjs/loader.js:661:22)
       at Function.Module._resolveFilename (internal/modules/cjs/loader.js:963:27)
       at Function.Module._load (internal/modules/cjs/loader.js:859:27)
       at Module.require (internal/modules/cjs/loader.js:1036:19)
       at require (internal/modules/cjs/helpers.js:72:18)
       at Object.<anonymous> (/Users/jaydenseric/Desktop/package-exports-test/test.js:2:1)
       at Module._compile (internal/modules/cjs/loader.js:1147:30) {
     code: 'ERR_INVALID_PACKAGE_TARGET'
   }
   

2. []:

   internal/modules/cjs/loader.js:587
     throw new ERR_INVALID_PACKAGE_TARGET(StringPrototypeSlice(baseUrl.pathname
     ^
   
   Error [ERR_INVALID_PACKAGE_TARGET]: Invalid "exports" target [] defined for './lib/helpers/' in the package config /Users/jaydenseric/Desktop/package-exports-test/node_modules/a/package.json
       at resolveExportsTarget (internal/modules/cjs/loader.js:587:13)
       at applyExports (internal/modules/cjs/loader.js:519:14)
       at resolveExports (internal/modules/cjs/loader.js:541:12)
       at Function.Module._findPath (internal/modules/cjs/loader.js:661:22)
       at Function.Module._resolveFilename (internal/modules/cjs/loader.js:963:27)
       at Function.Module._load (internal/modules/cjs/loader.js:859:27)
       at Module.require (internal/modules/cjs/loader.js:1036:19)
       at require (internal/modules/cjs/helpers.js:72:18)
       at Object.<anonymous> (/Users/jaydenseric/Desktop/package-exports-test/test.js:2:1)
       at Module._compile (internal/modules/cjs/loader.js:1147:30) {
     code: 'ERR_INVALID_PACKAGE_TARGET'
   }
   

Looks like they are both invalid :(

[jaydenseric]

I think null should be a valid value, to allow deliberate blacklisting.

[ljharb]

Right, but that means it throws - correctly - when you attempt to access the path?

[jaydenseric]

Sure it throws, but the ERR_INVALID_PACKAGE_TARGET error is from the perspective that the package exports is misconfigured, not that the user has imported something wrongly.

Even the fake path solution is not ideal in that regard, because the MODULE_NOT_FOUND error is actually different to the regular ERR_PACKAGE_PATH_NOT_EXPORTED error users would expect if they are trying import something private. They might look and see the file there in node_modules, and get frustrated because the file should be found.

[coreyfarrell]

Add a package.json inside lib with its own "exports".

To my knowledge "exports" is only applied from the package.json at the top-level of any package. I've experimented with putting package.json in sub-directories and was not able to get node.js to recognize it.

In node.js 13.10.0+ the following seems to produce the desired result:

  "name": "pkg1",
  "exports": {
    "./lib/": "./lib/",
    "./lib/helpers/": {}
  }

See https://github.com/coreyfarrell/pkg-exports for a demonstration of this, require('pkg1/lib/helpers/') gives the desired error in 13.10.0+ with code ERR_PACKAGE_PATH_NOT_EXPORTED. Earlier versions of node.js 13 give different but IMO still acceptable errors.

[guybedford]

Explicitly supporting null as indicating the ERR_PACKAGE_PATH_NOT_EXPORTED for trailing slash matches sounds like it would be a useful feature! I'd gladly approve a Node.js PR along these lines, and it should be relatively straightforward.

[guybedford]

Also, the empty [] should probably be giving a ERR_PACKAGE_PATH_NOT_EXPORTED as well, so perhaps that is something that should be fixed too.

[jkrems]

I think I'd prefer documenting {} as the canonical way of doing it. That way there's one pattern people have to learn and it'll hopefully easier to recognize in unfamiliar projects. I'm not sure null is fundamentally easier to understand than {} in this context.

Not going to block any PR or even discourage creating them. Just in terms of documentation, I think we should pick one. And if {} is already given valuable error messages, it seems nice to just go with it.

[guybedford]

I've created a PR for further discussion in nodejs/node#32838.