Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/sweet-mangos-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@arethetypeswrong/core": minor
---

New problem kind: **Missing `export =`**

Previously, `FalseExportDefault` had many cases of false positives where the JavaScript assigned an object to `module.exports`, and that object had a `default` property pointing back to itself. This pattern is not a true `FalseExportDefault`, but it is still problematic if the types only declare an `export default`. These kinds of false positives of `FalseExportDefault` are now instead reported as `MissingExportEquals`.

Additionally, `FalseExportDefault` was only ever reported as being visible in `node16-esm`, but this was incorrect. The consequences are most likely to be visible in `node16-esm`, but the problem is fundamentally independent of the module resolution mode, and inaccuracies can be observed in other modes as well, especially when `esModuleInterop` is not enabled.
77 changes: 77 additions & 0 deletions docs/problems/MissingExportEquals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# ❓ Missing `export =`

The JavaScript appears to set both `module.exports` and `module.exports.default` for improved compatibility, but the types only reflect the latter (by using `export default`). This will cause TypeScript under the `node16` module mode to think an extra `.default` property access is required, which will work at runtime but is not necessary. These types `export =` an object with a `default` property instead of using `export default`.

## Explanation

This problem occurs when a CommonJS JavaScript file appears to use a compatibility pattern like:

```js
class Whatever {
/* ... */
}
Whatever.default = Whatever;
module.exports = Whatever;
```

but the corresponding type definitions only reflect the existence of the `module.exports.default` property:

```ts
declare class Whatever {
/* ... */
}
export default Whatever;
```

The types should declare the existence of the `Whatever` class on both `module.exports` and `module.exports.default`. The method of doing this can vary depending on the kinds of things already being exported from the types. When the `export default` exports a class, and that class is the only export in the file, the `default` can be declared as a static property on the class, and the `export default` swapped for `export =`:

```ts
declare class Whatever {
static default: typeof Whatever;
/* ... */
}
export = Whatever;
```

When the file exports additional types, it will be necessary to declare a `namespace` that merges with the class and contains the exported types:

```ts
declare class Whatever {
static default: typeof Whatever;
/* ... */
}
declare namespace Whatever {
export interface WhateverProps {
/* ... */
}
}
export = Whatever;
```

This merging namespace can also be used to declare the `default` property when the main export is a function:

```ts
declare function Whatever(props: Whatever.WhateverProps): void;
declare namespace Whatever {
declare const _default: typeof Whatever;
export { _default as default };

export interface WhateverProps {
/* ... */
}
}
export = Whatever;
```

## Consequences

This problem is similar to the [“Incorrect default export”](./FalseExportDefault.md) problem, but in this case, the types are _incomplete_ rather than wholly incorrect. This incompleteness may lead TypeScript users importing from Node.js ESM code, or CommonJS code without `esModuleInterop` enabled, to add an extra `.default` property onto default imports to access the module’s `module.exports.default` property, even though accessing the `module.exports` would have been sufficient.

```ts
import Whatever from "pkg";
Whatever.default(); // Ok, but `Whatever()` would have worked!
```

## Common causes

This problem is usually caused by library authors incorrectly hand-authoring declaration files to match existing JavaScript rather than generating JavaScript and type declarations from TypeScript with `tsc`, or by using a third-party TypeScript emitter that adds an extra compatibility layer to TypeScript written with `export default`. Libraries compiling to CommonJS should generally avoid writing `export default` as input syntax.
1 change: 1 addition & 0 deletions packages/cli/src/problemUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const problemFlags: Record<ProblemKind, string> = {
FallbackCondition: "fallback-condition",
CJSOnlyExportsDefault: "cjs-only-exports-default",
FalseExportDefault: "false-export-default",
MissingExportEquals: "missing-export-equals",
UnexpectedModuleSyntax: "unexpected-module-syntax",
InternalResolutionError: "internal-resolution-error",
};
Expand Down
12 changes: 7 additions & 5 deletions packages/cli/test/snapshots/@[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ $ attw @[email protected] -f table-flipped

@vitejs/plugin-react v3.1.0

❓ The JavaScript appears to set both module.exports and module.exports.default for improved compatibility, but the types only reflect the latter (by using export default). This will cause TypeScript under the node16 module mode to think an extra .default property access is required, which will work at runtime but is not necessary. These types export = an object with a default property instead of using export default. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/MissingExportEquals.md

🎭 Import resolved to a CommonJS type declaration file, but an ESM JavaScript file. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md


┌────────────────────────┬───────────────────────────┬────────────────────────┬─────────┐
│ │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler │
├────────────────────────┼───────────────────────────┼────────────────────────┼─────────┤
│ "@vitejs/plugin-react" │ 🟢 │ 🟢 (CJS) │ 🎭 Masquerading as CJS │ 🟢 │
└────────────────────────┴───────────────────────────┴────────────────────────┴─────────┘
┌────────────────────────┬───────────────────────┬───────────────────────┬────────────────────────┬─────────┐
│ │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler │
├────────────────────────┼───────────────────────┼───────────────────────┼────────────────────────┼─────────┤
│ "@vitejs/plugin-react" │ ❓ Missing `export =` │ ❓ Missing `export =` │ 🎭 Masquerading as CJS │ 🟢 │
└────────────────────────┴───────────────────────┴───────────────────────┴────────────────────────┴─────────┘


```
Expand Down
14 changes: 7 additions & 7 deletions packages/cli/test/snapshots/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ Build tools:
- rollup@^2.44.0
- @rollup/plugin-typescript@^10.0.1

No problems found 🌟
❓ The JavaScript appears to set both module.exports and module.exports.default for improved compatibility, but the types only reflect the latter (by using export default). This will cause TypeScript under the node16 module mode to think an extra .default property access is required, which will work at runtime but is not necessary. These types export = an object with a default property instead of using export default. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/MissingExportEquals.md


┌───────┬────────┬───────────────────┬────────────────────────────┐
│ │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler │
├───────┼────────┼───────────────────┼────────────────────────────┤
│ "ajv" │ 🟢 │ 🟢 (CJS) │ 🟢 (CJS) │ 🟢
└───────┴────────┴───────────────────┴────────────────────────────┘
┌───────┬───────────────────────┬───────────────────────┬───────────────────────┬───────────────────────┐
│ │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler
├───────┼───────────────────────┼───────────────────────┼───────────────────────┼───────────────────────┤
│ "ajv" │ ❓ Missing `export =` │ ❓ Missing `export =` │ ❓ Missing `export =` │ ❓ Missing `export =`
└───────┴───────────────────────┴───────────────────────┴───────────────────────┴───────────────────────┘


```

Exit code: 0
Exit code: 1
4 changes: 2 additions & 2 deletions packages/cli/test/snapshots/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Build tools:
- typescript@^4.8.4
- rollup@^2.67.0

Wildcard subpaths cannot yet be analyzed by this tool. https://github.com/arethetypeswrong/arethetypeswrong.github.io/issues/40
🃏 Wildcard subpaths cannot yet be analyzed by this tool. https://github.com/arethetypeswrong/arethetypeswrong.github.io/issues/40

💀 Import failed to resolve to type declarations or JavaScript files. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/NoResolution.md

Expand All @@ -24,7 +24,7 @@ Build tools:
├─────────────────────────────────────────┼──────────────────────┼──────────────────────────────┼────────────────────┼────────────────────┤
│ "axios" │ 🟢 │ 🟢 (CJS) │ 🟢 (ESM) │ 🟢 │
├─────────────────────────────────────────┼──────────────────────┼──────────────────────────────┼────────────────────┼────────────────────┤
│ "axios/unsafe/*" │ Unable to check │ Unable to check │ Unable to check │ Unable to check │
│ "axios/unsafe/*" │ 🃏 Unable to check │ 🃏 Unable to check │ 🃏 Unable to check │ 🃏 Unable to check │
├─────────────────────────────────────────┼──────────────────────┼──────────────────────────────┼────────────────────┼────────────────────┤
│ "axios/unsafe/core/settle.js" │ 💀 Resolution failed │ ❌ No types │ ❌ No types │ ❌ No types │
│ │ │ ⚠️ ESM (dynamic import only) │ │ │
Expand Down
10 changes: 5 additions & 5 deletions packages/cli/test/snapshots/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ hexoid v1.0.0
❗️ The resolved types use export default where the JavaScript file appears to use module.exports =. This will cause TypeScript under the node16 module mode to think an extra .default property access is required, but that will likely fail at runtime. These types should use export = instead of export default. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseExportDefault.md


┌──────────┬────────┬───────────────────┬──────────────────────────────┬─────────┐
│ │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler │
├──────────┼────────┼───────────────────┼──────────────────────────────┼─────────┤
│ "hexoid" │ 🟢 │ 🟢 (CJS) │ ❗️ Incorrect default export │ 🟢
└──────────┴────────┴───────────────────┴──────────────────────────────┴─────────┘
┌──────────┬──────────────────────────────┬──────────────────────────────┬──────────────────────────────┬──────────────────────────────┐
│ │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler
├──────────┼──────────────────────────────┼──────────────────────────────┼──────────────────────────────┼──────────────────────────────┤
│ "hexoid" │ ❗️ Incorrect default export │ ❗️ Incorrect default export │ ❗️ Incorrect default export │ ❗️ Incorrect default export
└──────────┴──────────────────────────────┴──────────────────────────────┴──────────────────────────────┴──────────────────────────────┘


```
Expand Down
Loading