bug: using package exports fields can result in incompatible types for identical types

[EisenbergEffect]

Bug Report

🔎 Search Terms

• incompatible type

🕗 Version & Regression Information

This bug pertains to the new support for Node16 package exports.

• This is an unexpected compilation error.
• This occurs in TypeScript 4.7.3.
• I was unable to test this on prior versions because it pertains to a feature introduced in 4.7.

⏯ Playground Link

A playground link will not demonstrate the issue because it only occurs when importing a second package's exports.

💻 Code

This PR demonstrates a set of code changes to the @microsoft/fast-element library which results in a package that cannot be properly consumed from TypeScript: microsoft/fast#6087

The PR simply moves some code, which was originally part of the default export, and makes it an optional set of functionality exposed only through a special package export via @microsoft/fast-element/binding/two-way.

This is an important scenario for us and a dependency of our upcoming release.

🙁 Actual behavior

As a test, I built the package, which builds fine. Then I created a new project where I could consume the package. Since we haven't published this package yet (or even merged the PR), I made a quick check by simply copying my local version of the package over into the node_modules folder of my test project. I then attempted to import via the path above. There is no problem importing. The problem is that the types between the default export and the special export don't match, according to the compiler. So, if you have a function that takes a certain type as an input, coming from the default export, and then the special export exports an implementor of that interface, you cannot pass the instance to the function. Here is a made up example of what I'm talking about:

@foo/bar index.ts

export interface Task {
  run();
}

export function runTask(task: Task) {
  task.run();
}

@foo/bar cool-tasks.ts

import { Task } from "./index.js";

export const coolTask: Task = {
  run() {
    // ellided
  }
};

@foo/bar package.json

"exports": {
    ".": {
      "types": "./dist/index.d.ts",
      "production": "./dist/esm/index.js",
      "development": "./dist/esm/index.debug.js",
      "default": "./dist/esm/index.js"
    },
    "./cool-tasks": {
      "types": "./dist/dts/cool-tasks.d.ts",
      "default": "./dist/esm/cool-tasks.js"
    }
  }

Usage in a Project

import { runTask } from "@foo/bar";
import { coolTask } from "@foo/bar/cool-tasks";

runTask(coolTask); // Compiler error!!! Types are incompatible!

Note: I have not tried out this exact code, but this is essentially the pattern from our package with the PR linked above. We cannot pass the twoWay BindingConfig const to the bind function that takes an instance BindingConfig. TypeScript thinks that the types are incompatible, even though they are both referencing the same file in the package.

Here is a screenshot of the real code in action:

This exact same consuming code works perfectly if I simply move the twoWay code back into the default export and import it from there. If you examine the PR above, you will see that all we have done is moved the code to its own file and specified a package export.

🙂 Expected behavior

I would expect this to compile without issue.

[EisenbergEffect]

Ok, I have discovered a way to easily reproduce the error with code we have already shipped in beta. Here are the instructions:

• npm install --save @microsoft/[email protected]
• Attempt to compile the code below

import { ArrayObserver, Observable } from "@microsoft/fast-element";
import { mergeSpliceStrategy } from "@microsoft/fast-element/splice-strategies";

const observer = Observable.getNotifier<ArrayObserver>([]);
observer.strategy = mergeSpliceStrategy;

You will see the same type of error as above when attempting to set the strategy property. mergeSpliceStrategy is of type SpliceStrategy and the strategy property of the observer is also of type SpliceStrategy. The ArrayObserver and the mergeSpliceStrategy both internally import this interface from the same file. However, when the strategy implementation is imported into a new project from a special package export, TypeScript believes that the types are incompatible, again, even though they are referencing the same type and source code from the same package. If I move the strategy implementation code into the main package export, there is no error. However, that's not an option for this and several other features.

This is a major ship-blocker for FAST 2.0. We really appreciate your assistance in resolving this issue. Please let me know if there's anything I can do to help.

[weswigham]

This is likely because you have two, distinct copies of SpliceStrategy, relying on two distinct copies of the nominal SubscriberSet class (because of its private members) - one in ./dist/fast-element.d.ts (the one reachable via the types in the root import), and one in ./dist/dts/observation/array.d.ts (the one reachable via the deep import). You're essentially shipping two distinct packages worth of types.

If you have deep entrypoints, you really shouldn't be bundling you .d.ts file for your root entrypoint. Bundling your declaration files is only something you should do to hide deep entrypoints - if you're using export maps to explicitly expose specific deep entrypoints, you no longer have a good reason to bundle your declaration files, and it causes issues like this.

[EisenbergEffect]

@weswigham Are you saying that if we point our root d.ts export to the non-bundled index.d.ts then the problems with just "magically" disappear? In other words, do we just change this:

".": {
      "types": "./dist/fast-element.d.ts",
      "production": "./dist/esm/index.js",
      "development": "./dist/esm/index.debug.js",
      "default": "./dist/esm/index.js"
    },

Into this:

".": {
      "types": "./dist/dts/index.d.ts", <-- just change this?
      "production": "./dist/esm/index.js",
      "development": "./dist/esm/index.debug.js",
      "default": "./dist/esm/index.js"
    },

[weswigham]

Yep. (And as I've said, you probably don't have a great reason to even ship bundled declarations at all now that you're shipping multiple explicit entrypoints)

[EisenbergEffect]

Thanks @weswigham! I'll give this a try. It makes sense. I'll report back soon.

[EisenbergEffect]

A quick local test (updating some package.json in node modules folder) shows that this indeed gets it working beautifully. Again, huge thanks @weswigham !!! Closing issue as resolved.

[EisenbergEffect]

@weswigham QQ: If I'm using exports as above, should I also remove the "types" package.json field that previous also pointed to the rollup dts? Am I right in assuming that with exports configuration that the "types" field is no longer needed? Referring specifically to the "type" field at the package level, not the ones in the exports.

[weswigham]

Keep the types field for people running older versions of node/ts, but you're free to point it at the unbundled entry point, too.

[EisenbergEffect]

Thank you!