fix exports in package.json to work with ts's implementation of node16 moduleResolution

[lerouxb]

This is a follow-up to #546 where I didn't properly understand this.

The context of why/where I need this is to be able to use a separate types export from the default one from mongosh.

The latest error I'm getting there:

src/index.ts:428:5 - error TS7016: Could not find a declaration file for module '@mongodb-js/mongodb-ts-autocomplete'. '/Users/leroux.bodenstein/mongo/mongosh/packages/autocomplete/node_modules/@mongodb-js/mongodb-ts-autocomplete/dist/.esm-wrapper.mjs' implicitly has an 'any' type.
  There are types at '/Users/leroux.bodenstein/mongo/mongosh/packages/autocomplete/node_modules/@mongodb-js/mongodb-ts-autocomplete/dist/index.d.ts', but this result could not be resolved when respecting package.json "exports". The '@mongodb-js/mongodb-ts-autocomplete' library may need to update its package.json or typings.

428     '@mongodb-js/mongodb-ts-autocomplete'
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Basically TS sees the right types but refuses to use it 😛

The way this should work is actually, if you're exporting multiple things:

exports: {
  '.': {
    import: {
      default: '',
      types: '',
    },
    require: {
      default: '',
      types: '',
    }
  }
  foo: {
    import: {
      default: '',
      types: '',
    },
    require: {
      default: '',
      types: '',
    }
  }
}

then if you're just exporting one thing (the usual case):

exports: {
  import: {
    default: '',
    types: '',
  },
  require: {
    default: '',
    types: '',
  }
}

Then there's also "main" and "types" for things that don't support "exports" yet.

References:

• https://www.typescriptlang.org/tsconfig/#moduleResolution (not much there, unfortunately)
• Types that are found, but not exported in package.json, should still be usable microsoft/TypeScript#52363 (comment) and the subsequent comment.
• https://github.com/addaleax/gen-esm-wrapper

[lerouxb]

I moved main and types together because logically that's how it works. Systems that import code will use those if they aren't using exports.

[lerouxb]

I don't think anything that's going to use exports.require is actually going to pick up this types field because the commonjs moduleResolution in typescript was clearly happily using "types" from the root and now with node16 it will use import.types. plus node/js (outside of typescript) is never going to import the types anyway but it is here for consistency.

[kraenhansen]

A "types" condition like this is redundant, since TSC will automatically infer it from the .js file.

Suggested change

"types": "./dist/index.d.ts"

[gagik]

A "types" condition like this is redundant, since TSC will automatically infer it from the .js file.

is this the case? I have had issues with packages which don't explicitly define it before

[gagik]

I guess that'd work if you do "require": "./dist/index.js",
but not "require": {default: "./dist/index.js",}?

[lerouxb]

I'm not sure about this because if someone does use the commonjs module resolution method (or something in between) it would work if it picks both main and types from the root, but what if it picks exports.require? We'd basically rely on it falling back to the types field at the root? I don't think we want types inferred if there are proper types, do we?

[lerouxb]

oh wait I think I'm misunderstanding. You mean the top-level one since ts will be using the exports anyway? I suspect you're right.

[kraenhansen]

Chatted with @lerouxb offline, I'd be happy to share an example for anyone interested.

[lerouxb]

This comment here says that you need separate types files for require/import but so far in my testing by just npm linking things together it seems to work?

What I'll do once this PR lands is to make sure we bump all these packages where they are used so that I find and fix any errors and not someone else.

If we do need a types files in a different format, then I guess we'll have to change https://github.com/addaleax/gen-esm-wrapper

[lerouxb]

I updated the create-workspace template as well.

[coveralls]

coverage: 72.404%. remained the same
when pulling fd35887 on fix-types-for-real
into 98a0efb on main.

[kraenhansen]

A "types" condition like this is redundant, since TSC will automatically infer it from the .js file.

Suggested change

	"require": {
	"default": "./dist/index.js",
	"types": "./dist/index.d.ts"
	},
	"require": "./dist/index.js",

[lerouxb]

so just for the "require" ones? I left a comment elsewhere where I already had a suspicion like that.

[kraenhansen]

Are all the consumers of these packages updated? In which case we could keep things simple and just delete the "main" and "types", as per Node.js docs:

The "exports" provides a modern alternative to "main" allowing multiple entry points to be defined, conditional entry resolution support between environments, and preventing any other entry points besides those defined in "exports". This encapsulation allows module authors to clearly define the public interface for their package.

For new packages targeting the currently supported versions of Node.js, the "exports" field is recommended. For packages supporting Node.js 10 and below, the "main" field is required. If both "exports" and "main" are defined, the "exports" field takes precedence over "main" in supported versions of Node.js.

Suggested change

	"main": "dist/index.js",
	"types": "./dist/index.d.ts",

[lerouxb]

Yeah I was thinking about that. I don't know if I can tell 100% before just removing these. And if it doesn't "just work" I'd have to upgrade all our repos at once. I was thinking I can remove those in a separate PR and then if it all blows up I only have to revert that one PR.

[lerouxb]

I'm gonna merge this because I'm kinda blocked on it right now, but I'll keep speaking to @kraenhansen about his feedback and come around to it.