docs: added JS doc comments and tsconfig

srijan-paul commented 2 years ago

Added JS doc comments and a TSConfig that should generate the type definitions. Note that it still may require changes. Things that I'm curious about:

Should we be testing these type definitions?
Are the AST returned by espree and acorn similar enough that using acorn.Node as the return type for espree.parse is safe?

eslint-github-bot[bot] commented 2 years ago

Hi @srijan-paul!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot[bot] commented 2 years ago

Hi @srijan-paul!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

eslint-github-bot[bot] commented 2 years ago

Hi @srijan-paul!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

srijan-paul commented 2 years ago

Whoops, I missed adding the build file to package.json. Fixed!

mdjermanovic commented 2 years ago

Should we be testing these type definitions?

I think that would be good. Otherwise, we won't know if the definitions are broken, which would make Espree unusable in typescript projects?

Speaking of that, npm run build produces the following espree.d.ts:

/**
 * Tokenizes the given code.
 * @param {string} code The code to tokenize.
 * @param {ParserOptions} options Options defining how to tokenize.
 * @returns {EsprimaToken[]} An array of tokens.
 * @throws {SyntaxError} If the input code is invalid.
 * @private
 */
export function tokenize(code: string, options: ParserOptions): EsprimaToken[];
/**
 * Parses the given code.
 * @param {string} code The code to tokenize.
 * @param {ParserOptions} options Options defining how to tokenize.
 * @returns {acorn.Node} The "Program" AST node.
 * @throws {SyntaxError} If the input code is invalid.
 */
export function parse(code: string, options: ParserOptions): acorn.Node;
export const version: "main";
export const Syntax: {};
export const VisitorKeys: any;
export const latestEcmaVersion: number;
export const supportedEcmaVersions: number[];
export type acorn = typeof acorn;
export type ParserOptions = import("./lib/options").ParserOptions;
export type EsprimaToken = import("./lib/token-translator").EsprimaToken;
export type TokenRange = import("./lib/token-translator").TokenRange;
import * as acorn from "acorn";
//# sourceMappingURL=espree.d.ts.map

When I open espree.d.ts in VSCode, it reports one error:

Namespace '"d:/tmp/espree-types/espree/dist/lib/token-translator"' has no exported member 'TokenRange'.

Also, when I try to use this version from another project (I added "types": "dist/espree.d.ts" in node_modules/espree/package.json, and copied all published files including dist/espree.d.ts), those relative paths seems to be a problem for tsc:

//----------- test.ts ---------------
import * as espree from "espree";

> tsc

node_modules/espree/dist/espree.d.ts:24:36 - error TS2307: Cannot find module './lib/options' or its corresponding type declarations.

24 export type ParserOptions = import("./lib/options").ParserOptions;
                                      ~~~~~~~~~~~~~~~

node_modules/espree/dist/espree.d.ts:25:35 - error TS2307: Cannot find module './lib/token-translator' or its corresponding type declarations.

25 export type EsprimaToken = import("./lib/token-translator").EsprimaToken;
                                     ~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/espree/dist/espree.d.ts:26:33 - error TS2307: Cannot find module './lib/token-translator' or its corresponding type declarations.

26 export type TokenRange = import("./lib/token-translator").TokenRange;
                                   ~~~~~~~~~~~~~~~~~~~~~~~~

Found 3 errors.

mdjermanovic commented 2 years ago

Also, when I try to use this version from another project (I added "types": "dist/espree.d.ts" in node_modules/espree/package.json, and copied all published files including dist/espree.d.ts), those relative paths seems to be a problem for tsc:

//----------- test.ts ---------------
import * as espree from "espree";

> tsc

node_modules/espree/dist/espree.d.ts:24:36 - error TS2307: Cannot find module './lib/options' or its corresponding type declarations.

24 export type ParserOptions = import("./lib/options").ParserOptions;
                                      ~~~~~~~~~~~~~~~

node_modules/espree/dist/espree.d.ts:25:35 - error TS2307: Cannot find module './lib/token-translator' or its corresponding type declarations.

25 export type EsprimaToken = import("./lib/token-translator").EsprimaToken;
                                     ~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/espree/dist/espree.d.ts:26:33 - error TS2307: Cannot find module './lib/token-translator' or its corresponding type declarations.

26 export type TokenRange = import("./lib/token-translator").TokenRange;
                                   ~~~~~~~~~~~~~~~~~~~~~~~~

Found 3 errors.

tsc actually creates dist/espree.d.ts and dist/lib/ with a .d.ts file for each .js file from lib/:

.
└── dist/
    ├── espree.cjs 
    ├── espree.cjs.map
    ├── espree.d.ts
    ├── espree.d.ts.map
    └── lib/
        ├── ast-node-types.d.ts
        ├── ast-node-types.d.ts.map
        ├── espree.d.ts
        ├── espree.d.ts.map
        ├── features.d.ts
        ├── features.d.ts.map
        ├── options.d.ts
        ├── options.d.ts.map
        ├── token-translator.d.ts
        ├── token-translator.d.ts.map
        ├── version.d.ts
        └── version.d.ts.map

Should we then also add "dist/lib" to "files" in package.json?

brettz9 commented 2 years ago

Also, when I try to use this version from another project (I added "types": "dist/espree.d.ts" in node_modules/espree/package.json, and copied all published files including dist/espree.d.ts), those relative paths seems to be a problem for tsc:

//----------- test.ts ---------------
import * as espree from "espree";

> tsc

node_modules/espree/dist/espree.d.ts:24:36 - error TS2307: Cannot find module './lib/options' or its corresponding type declarations.

24 export type ParserOptions = import("./lib/options").ParserOptions;
                                      ~~~~~~~~~~~~~~~

node_modules/espree/dist/espree.d.ts:25:35 - error TS2307: Cannot find module './lib/token-translator' or its corresponding type declarations.

25 export type EsprimaToken = import("./lib/token-translator").EsprimaToken;
                                     ~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/espree/dist/espree.d.ts:26:33 - error TS2307: Cannot find module './lib/token-translator' or its corresponding type declarations.

26 export type TokenRange = import("./lib/token-translator").TokenRange;
                                   ~~~~~~~~~~~~~~~~~~~~~~~~

Found 3 errors.

tsc actually creates dist/espree.d.ts and dist/lib/ with a .d.ts file for each .js file from lib/:

.
└── dist/
    ├── espree.cjs 
    ├── espree.cjs.map
    ├── espree.d.ts
    ├── espree.d.ts.map
    └── lib/
        ├── ast-node-types.d.ts
        ├── ast-node-types.d.ts.map
        ├── espree.d.ts
        ├── espree.d.ts.map
        ├── features.d.ts
        ├── features.d.ts.map
        ├── options.d.ts
        ├── options.d.ts.map
        ├── token-translator.d.ts
        ├── token-translator.d.ts.map
        ├── version.d.ts
        └── version.d.ts.map

Should we then also add "dist/lib" to "files" in package.json?

TypeScript allows an outFile option. Maybe that would prevent the need?

srijan-paul commented 2 years ago

Will update this over the weekend. Got caught with life there.

srijan-paul commented 2 years ago

Updated the PR (squashed some commits). Should be ready to go now :)

@mdjermanovic I've fixed the issue with incorrectly generated .d.ts. I'm not sure what the best way would be to test the d.ts files though. Suggestions welcome. @brettz9 Thanks for pointing out the issues with comments. Fixed.

srijan-paul commented 2 years ago

Hya, not to rush anyone, but I was wondering if I could get some feedback on this PR since I'd be needing the type definitions in a typescript project I'm working on :p

nzakas commented 2 years ago

@brettz9 @mdjermanovic any followups here?

mdjermanovic commented 2 years ago

I'm still seeing the problems described in https://github.com/eslint/espree/pull/530#issuecomment-1025766581:

import("./lib/token-translator").TokenRange - TokenRange type doesn't exist in that module.
The published declaration file imports declaration files that are not published (you can run npm pack to generate a .tgz file, and then install Espree from that .tgz file in a TypeScript project to test).

mdjermanovic commented 2 years ago

I'm not sure what the best way would be to test the d.ts files though. Suggestions welcome.

Maybe we could use tsd, like in https://github.com/eslint/eslint-visitor-keys

mdjermanovic commented 2 years ago

Do we want to export types named EsprimaToken and EsprimaComment, or would it be better to name them just Token and Comment?

nzakas commented 2 years ago

Token and Comment are probably fine

linux-foundation-easycla[bot] commented 2 years ago

The committers listed above are authorized under a signed CLA.

:white_check_mark: login: srijan-paul / name: injuly (f8ce0d2b90b8da2f4f87074e41a5158ae4ecd011, 41d48d8a7f72ac4c33fffa3a37dac7c426b8201f, 625af0c8fbaedbd443def792246a2c140b3ba833)

srijan-paul commented 2 years ago

Ack. Looking for a way to fix the issue with broken imports in the published package. EDIT: adding dist/lib to files in package.json fixed the issue.

btmills commented 2 years ago

Hi @srijan-paul, thank you for your work on this! Since this and #544 are attacking the same problem but #544 additionally enables type checking in the repository, we'd like to focus our efforts on #544. Please know that we appreciate your work on this so far, and if you haven't already heard from him, nzakas will be reaching out because this is exactly the sort of improvement that our contributor pool payments were designed for. I'm sorry this change won't be getting merged, but I hope to see what we've learned from this work reflected in #544 and would welcome seeing your feedback there!

eslint / espree

docs: added JS doc comments and tsconfig #530