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

d.ts files are getting ignored when entrypoint is a js file #370

Open
teidesu opened this issue Apr 9, 2024 · 6 comments
Open

d.ts files are getting ignored when entrypoint is a js file #370

teidesu opened this issue Apr 9, 2024 · 6 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@teidesu
Copy link

teidesu commented Apr 9, 2024

which leads to jsr resorting to trying to infer types trivially, and as a result - failure to use the library altogether.

mcve

deno.json

{
    "name": "@foo/bar",
    "version": "1.3.5",
    "exports": "./mod.js"
}

mod.js

const exports = {};

exports.ns = {};
(function(ns) {
    ns.bar = "egg";
})(exports.ns);

export const ns = exports.ns;

mod.d.ts

export declare namespace ns {
    const bar = "egg";
}

the above is similar to commonjs output from tsc for typescript namespaces

expected result

jsr uses d.ts files for typings and doesn't try to infer them on its own

actual result

jsr warns about "slow types" when publishing and advises to add d.ts files (which are already there):

warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoint
 --> /path/to/mod.js
  = hint: add a type declaration (d.ts) for the JavaScript module, or rewrite it to TypeScript

  info: JavaScript files with no corresponding declaration require type inference to be type checked
  info: fast check avoids type inference, so JavaScript entrypoints should be avoided
  docs: https://jsr.io/go/slow-type-unsupported-javascript-entrypoint

btw, linked docs might be out of date? there's nothing mentioned about js entrypoints

additional context

due to this bug, export const ns from the mcve, whose type can't be trivially inferred, results being unusable at all:

// some other package
import { ns } from '@foo/bar'

console.log(ns.bar) // errors here, ns is not a namespace
@github-project-automation github-project-automation bot moved this to Needs Triage in JSR Apr 9, 2024
@lucacasonato
Copy link
Member

In Deno, you have to explicitly link your .d.ts files to your .js files because Deno's module resolution algorithm does not probe for sibling files. See https://docs.deno.com/runtime/manual/advanced/typescript/types#using-the-triple-slash-reference-directive.

In your case, you want to add a /// <reference types="./mod.d.ts" /> to your JS file.

@teidesu
Copy link
Author

teidesu commented Apr 9, 2024

i believe error message/docs could be improved then.
this difference is currently not mentioned anywhere in jsr specifically, so coming from node background this behavior is pretty unexpected.

adding /// reference does fix it, thanks!

@lucacasonato
Copy link
Member

Yes - we'll improve the docs and error message.

@lucacasonato lucacasonato added the documentation Improvements or additions to documentation label Apr 10, 2024
@lucacasonato lucacasonato moved this from Needs Triage to Ready in JSR Apr 10, 2024
@lucacasonato lucacasonato added bug docs gen Related to the documentation generation documentation Improvements or additions to documentation and removed documentation Improvements or additions to documentation bug docs gen Related to the documentation generation labels Apr 19, 2024
@nzakas
Copy link
Contributor

nzakas commented May 9, 2024

@lucacasonato I just ran into this as well. Would you consider adding a types field to jsr.json specifically for JS entrypoints to specify their .d.ts files?

The reason is that this is a bit of a mess to implement for each package, as including the triple-slash directive before tsc runs causes errors, so my process ends up like this:

  1. Run Rollup
  2. Run tsc on the rolled up file
  3. Prepend triple-slash directive to file

I understand that Deno doesn't crawl around the directory looking for .d.ts files, so maybe allowing us to explicitly specify the type declarations file in jsr.json could improve the developer experience?

@lucacasonato
Copy link
Member

You can now specify // @ts-self-types="./types.d.ts" instead of the triple slash reference comment. This doesn't break TypeScript so you don't need to do it after type checking :)

@nzakas
Copy link
Contributor

nzakas commented May 10, 2024

Ooh thank you! I will give that a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Ready
Development

No branches or pull requests

4 participants