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

fix TypeScript support #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

nazar-pc
Copy link

@nazar-pc nazar-pc commented Jul 3, 2019

Fixes #16

@mysticatea
Copy link
Owner

Thank you for your contribution.

Why is the types field necessary? I have been using this package with TypeScript, but I have never experienced any problem.

@nazar-pc
Copy link
Author

nazar-pc commented Jul 4, 2019

Well, I'm using TypeScript 3.5.2 and it didn't work until I added declarations manually.

@mysticatea
Copy link
Owner

I'm using TypeScript 3.5.2, too, but I couldn't reproduce it. The tsc command works fine and VSCode recognizes abort-controller/dist/abort-controller.d.ts.

Would you provide repro steps?

@nazar-pc
Copy link
Author

nazar-pc commented Jul 4, 2019

IntelliJ IDE doesn't offer any autocomplete and I'm using relatively strict mode in TypeScript, will share tsconfig.json a bit later.

@nazar-pc
Copy link
Author

nazar-pc commented Jul 4, 2019

My tsconfig.json looks like this:

{
  "compilerOptions": {
    "module": "commonjs",
    "target": "es6",
    "sourceMap": true,
    "forceConsistentCasingInFileNames": true,
    "noImplicitReturns": true,
    "moduleResolution": "node",
    "allowSyntheticDefaultImports": false,
    "sourceRoot": "src",
    "baseUrl": "src",
    "outDir": "dist",
    "strict": true,
    "noUnusedParameters": true,
    "noUnusedLocals": true,
    "lib": [
      "es2015"
    ]
  },
  "include": [
    "src"
  ]
}

@mysticatea
Copy link
Owner

Sorry, I still couldn't reproduce it. The tsc command doesn't show any errors.

A console log
~\dev\sandbox> cat test.ts
import { AbortController } from "abort-controller"

const controller = new AbortController()
controller.signal.addEventListener("abort", () => {
    console.log("Aborted.")
})
controller.abort()
~\dev\sandbox> cat tsconfig.json
{
    "compilerOptions": {
      "module": "commonjs",
      "target": "es6",
      "sourceMap": true,
      "forceConsistentCasingInFileNames": true,
      "noImplicitReturns": true,
      "moduleResolution": "node",
      "allowSyntheticDefaultImports": false,
      "sourceRoot": "src",
      "baseUrl": "src",
      "outDir": "dist",
      "strict": true,
      "noUnusedParameters": true,
      "noUnusedLocals": true,
      "lib": [
        "es2015"
      ]
    },
    "include": [
      "test.ts"
    ]
  }
~\dev\sandbox> tsc
~\dev\sandbox> cat test.js
"use strict";
exports.__esModule = true;
var abort_controller_1 = require("abort-controller");
var controller = new abort_controller_1.AbortController();
controller.signal.addEventListener("abort", function () {
    console.log("Aborted.");
});
controller.abort();

@MrJohz
Copy link

MrJohz commented Jul 25, 2019

Hi, I just wanted to echo @nazar-pc's problem here (I'm also running into it), and also point at the TypeScript documentation for this situation, which is here. I'm intrigued why tsc can handle this case, but officially the correct way to provide consumable types for a package written in TypeScript is to provide the types field in the package.json.

@nazar-pc
Copy link
Author

@mysticatea, why is this not merged yet?
It is such a trivial change and there is literally no place for discussion because it directly matches official TypeScript documentation.

@jstewmon
Copy link

jstewmon commented Apr 9, 2020

I think this problem is specific to IntelliJ. I'm only able to reproduce the problem with IntelliJ - both VSCode and tsc resolve the type definitions.

According to the typescript handbook:

TypeScript will mimic the Node.js run-time resolution strategy in order to locate definition files for modules at compile-time. To accomplish this, TypeScript overlays the TypeScript source file extensions (.ts, .tsx, and .d.ts) over Node’s resolution logic. TypeScript will also use a field in package.json named "types" to mirror the purpose of "main" - the compiler will use it to find the “main” definition file to consult.

Since abort-controller/package.json has "main": "dist/abort-controller", TypeScript will find dist/abort-controller.d.ts.

I tried searching YouTrack for an issue, but didn't find one, so it may be worth reporting to JetBrains if this issue is important to you.

FWIW, IntelliJ will find the type definitions if you provide a path mapping in your tsconfig:

{
  "compilerOptions": {
    "paths": {
      "abort-controller": [
        "./node_modules/abort-controller/dist/abort-controller.d.ts"
      ]
    }
  }
}

@nazar-pc
Copy link
Author

nazar-pc commented Apr 9, 2020

Why tho? It only takes to add 1 line in package.json just like TypeScript documentation suggests and most other packages already do.

@jstewmon
Copy link

jstewmon commented Apr 9, 2020

🤷 If it were me, I would probably just add it...

But, I think @mysticatea is reasonable in taking the principled position that this package conforms to TypeScript's module resolution rules, and it is not a maintainer's responsibility to work around bugs in discretionary tools chosen by the community.

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

Successfully merging this pull request may close these issues.

TypeScript support is broken
4 participants