Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate .d.mts types for imports #2472

Closed
fourdim opened this issue Jul 30, 2023 · 8 comments · Fixed by #2677
Closed

Generate .d.mts types for imports #2472

fourdim opened this issue Jul 30, 2023 · 8 comments · Fixed by #2677
Labels
bug Something is not working.

Comments

@fourdim
Copy link

fourdim commented Jul 30, 2023

Describe the bug

Upgrading @floating-ui/dom from version 1.2.9 to 1.50 will give following errors:

src/index.ts:6:42 - error TS7031: Binding element 'x' implicitly has an 'any' type.

6 computePosition(button, tooltip).then(({ x, y }) => {
                                           ~

src/index.ts:6:45 - error TS7031: Binding element 'y' implicitly has an 'any' type.

6 computePosition(button, tooltip).then(({ x, y }) => {

To Reproduce
https://codesandbox.io/p/sandbox/fancy-moon-t5p4ps?file=%2Fpackage.json%3A12%2C1-12%2C31

Steps to reproduce the behavior:

  1. Run yarn build
  2. See error

Expected behavior
No errors.

Screenshots
If applicable, add screenshots to help explain your problem.

image

Context:

  • Version 1.5.0
@atomiks
Copy link
Collaborator

atomiks commented Jul 30, 2023

In v1.5.0 the exports changed to put the types field below import and use the .mts extension to fix the Masquerading as CJS issue:

From:

  "types": "./src/types.d.ts",
  "exports": {
    "./package.json": "./package.json",
    ".": {
      "types": "./src/types.d.ts",
      "import":  "./dist/floating-ui.core.mjs",
      "module": "./dist/floating-ui.core.esm.js",
      "default": "./dist/floating-ui.core.umd.js"
    }
  },

To:

  "types": "./src/types.d.ts",
  "exports": {
    "./package.json": "./package.json",
    ".": {
      "import": {
        "types": "./src/types.d.mts",
        "default": "./dist/floating-ui.core.mjs"
      },
      "types": "./src/types.d.ts",
      "module": "./dist/floating-ui.core.esm.js",
      "default": "./dist/floating-ui.core.umd.js"
    }
  },

Which publint marks as "All good".

However, it looks like Are the types wrong? fails to resolve the types from ESM.

I don't really understand how to fix this correctly.

@atomiks atomiks added bug Something is not working. and removed NEEDS: triage labels Jul 30, 2023
@atomiks
Copy link
Collaborator

atomiks commented Jul 30, 2023

Seems like the issue is the .d.mts declaration files require extensions when importing which is currently missing.

@atomiks
Copy link
Collaborator

atomiks commented Jul 30, 2023

Ok so I think adding the extensions to the .d.mts files is going to require a bit of work where every internal module in the project needs to add .js/.ts when importing, then a build step to rewrite the .d.mts files to add the m prefix to any imports.

For now, I'll simply revert the .d.mts declarations since "masquerading as CJS" is not really that bad, and keep this issue open to track getting them to work correctly.

@fourdim
Copy link
Author

fourdim commented Jul 30, 2023

In my point of view, maybe you need to first tsc with emitDeclarationOnly: false and then use the generated js as the input of rollup. It is much easier to configure it if you use vite build system for the whole project ( as it will automatically do it for you by plugins ).

@fourdim
Copy link
Author

fourdim commented Jul 30, 2023

I'm working on a pull request to replace rollup with vite.

@atomiks atomiks changed the title tsc cannot use the declaration files in a correct way Generate .d.mts types for imports Nov 7, 2023
@bsunderhus
Copy link
Contributor

bsunderhus commented Dec 19, 2023

Just to confirm here @atomiks, this would be the ideal?

  "types": "./src/types.d.ts",
  "exports": {
    "./package.json": "./package.json",
    ".": {
      "import": {
        "types": "./src/types.d.mts",
        "default": "./dist/floating-ui.core.mjs"
      },
      "types": "./src/types.d.ts",
      "module": "./dist/floating-ui.core.esm.js",
      "default": "./dist/floating-ui.core.umd.js"
    }
  },

Seems like the generation of .d.mts files are identical to the on of .d.ts files, so why not just having a ctrl-c + ctrl-v task to copy "./src/types.d.ts" to "./src/types.d.mts"? am I missing something here?

@atomiks
Copy link
Collaborator

atomiks commented Dec 19, 2023

I did do that originally, but that's what led to this exact error in this issue :D Specifically: #2472 (comment)

The CJS type defs don't need extensions in imports (and are compiled from the source without them), but ESM ones do. import type {X} from './types.mjs';

And yes, this part is set up to replace it with the "m" eventually even though it's unneeded currently:

      "import": {
        "types": "./src/types.d.mts", // <----
        "default": "./dist/floating-ui.core.mjs"
      },

I also had trouble changing the tsconfig type resolution to use "bundler" or "NodeNext" in this repo. Only node works, which might be related somehow.

@bsunderhus
Copy link
Contributor

I think I got a possible solution actually, if we flatten the types into a single file using something like api-extractor then there will be no relative import to worry about this: import type {X} from './types.mjs'; vs import type {X} from './types';, since those imprts will be sliced out of the flat .d.ts file, in that case I guess there would be no difference between a .d.ts and .d.mts 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants