Skip to content

docs(ast-spec): add script to generates code strings for the doc files #3248

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

Closed
wants to merge 2 commits into from

Conversation

bradzacher
Copy link
Member

@bradzacher bradzacher commented Mar 30, 2021

I'm not sure if we will want to actually use this or not.
This was just an idea I had to allow us to embed the relevant code samples directly in a .md file saving consumers from having to switch between the .ts file and the .md file.

We could ofc just embed the file itself, but that's not as nice because:

  • there's the extra cruft of export keywords.
  • there's the extra cruft of the import statements.
  • some nodes are a union of different types for different usecases.
    • eg ClassProperty is a union of two types ClassPropertyComputedName and ClassPropertyNonComputedName
  • some nodes extend base nodes in other files
    • eg for the same ClassProperty case both the types extend base classes of similar names.
    • these types are pretty impossible to understand from the github UI (but are easy to interrogate from an IDE).

We may end up just ditching this... but it was worth putting it up for posterity.

As an example - this script generates the following output for ClassProperty:

interface ClassPropertyComputedName extends BaseNode {
  type: 'ClassProperty';
  key: Expression;
  computed: true;
  value: Expression | null;
  static: boolean;
  declare: boolean;
  readonly?: boolean;
  decorators?: Decorator[];
  accessibility?: "private" | "protected" | "public";
  optional?: boolean;
  definite?: boolean;
  typeAnnotation?: TSTypeAnnotation;
}

interface ClassPropertyNonComputedName extends BaseNode {
  type: 'ClassProperty';
  key: Identifier | NumberLiteral | StringLiteral;
  computed: false;
  value: Expression | null;
  static: boolean;
  declare: boolean;
  readonly?: boolean;
  decorators?: Decorator[];
  accessibility?: "private" | "protected" | "public";
  optional?: boolean;
  definite?: boolean;
  typeAnnotation?: TSTypeAnnotation;
}

type ClassProperty =
  | ClassPropertyComputedName
  | ClassPropertyNonComputedName;

@bradzacher bradzacher added DO NOT MERGE PRs which should not be merged yet documentation Documentation ("docs") that needs adding/updating labels Mar 30, 2021
@typescript-eslint
Copy link
Contributor

Thanks for the PR, @bradzacher!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@bradzacher bradzacher changed the title feat: refactor to split AST specification out as its own module docs(ast-spec): add script to generates code strings for the doc files Mar 30, 2021
@bradzacher bradzacher force-pushed the separate-spec-doc-generator branch from d061042 to b00a8cc Compare March 30, 2021 21:39
Fixes #2726
Fixes #2912

This PR is the basis for a big cleanup and reorganisation of the AST.
This first step takes the file `types/src/ts-estree.ts` and splits it up in its entirety.
This file was a monolith at 1700 lines - meaning it was a pain to organise and manage, and there was no way to isolate/restrict certain things (aside from adding comments).

This PR should ultimately be a no-op - there should be no breaking changes here.
I did fix up a number of types which I found when organising files into their folders.

Whilst this PR ultimately creates more LOC, the isolation enables a few things:
- By splitting the AST into its own module, it's isolated so easier to manage / govern
- By splitting each AST node into its own folder we can cleanly document and link to individual node specs
- By grouping nodes decls by folder, it's easier to inspect the types to validate unions are correct.
    - I found a number of invalid nodes in unions in this PR which have been fixed.
- In a future PR we can:
    - Add lint rule(s) to validate unions are correct (eg ensure all `Expression` types are included in the `Expression` union).
    - Easily add documentation about the node without cluttering things up
    - Colocate fixtures/snapshots with the node specs to document the cases that we expect a node to show up
    - Colocate the conversion logic here so that it's easier to validate that the spec and the conversion logic are in sync
        - This will make it much easier to implement and maintain #1852
I'm not sure if we will want to actually use this or not.
This was just an idea I had to allow us to embed the relevant code samples directly in a `.md` file saving consumers from having to switch between the .ts file and the .md file.

We could ofc just embed the file itself, but that's not as elegant as there's the extra cruft of `export` and  `import`s.
May end up just ditching this... but it was worth putting it up for posterity.

As an� example - this script generates the following output for `ClassProperty.ts`:

```ts
interface ClassPropertyComputedName extends BaseNode {
  type: 'ClassProperty';
  key: Expression;
  computed: true;
  value: Expression | null;
  static: boolean;
  declare: boolean;
  readonly?: boolean;
  decorators?: Decorator[];
  accessibility?: "private" | "protected" | "public";
  optional?: boolean;
  definite?: boolean;
  typeAnnotation?: TSTypeAnnotation;
}

interface ClassPropertyNonComputedName extends BaseNode {
  type: 'ClassProperty';
  key: Identifier | NumberLiteral | StringLiteral;
  computed: false;
  value: Expression | null;
  static: boolean;
  declare: boolean;
  readonly?: boolean;
  decorators?: Decorator[];
  accessibility?: "private" | "protected" | "public";
  optional?: boolean;
  definite?: boolean;
  typeAnnotation?: TSTypeAnnotation;
}

type ClassProperty =
  | ClassPropertyComputedName
  | ClassPropertyNonComputedName;
```
@bradzacher bradzacher force-pushed the separate-spec-doc-generator branch from b00a8cc to f1b42da Compare March 30, 2021 21:41
@bradzacher bradzacher force-pushed the separate-spec branch 2 times, most recently from 3f04cd5 to a5f8933 Compare May 4, 2021 22:18
Base automatically changed from separate-spec to master May 4, 2021 23:15
@bradzacher bradzacher added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Mar 4, 2022
@bradzacher bradzacher closed this Mar 4, 2022
@bradzacher bradzacher deleted the separate-spec-doc-generator branch March 4, 2022 07:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DO NOT MERGE PRs which should not be merged yet documentation Documentation ("docs") that needs adding/updating stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant