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.

Sign up for GitHub

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

Jump to bottom

feat(allow-scripts): generate & ship typescript declarations #617

Closed
wants to merge 1 commit into from
Closed

feat(allow-scripts): generate & ship typescript declarations #617

wants to merge 1 commit into from

Conversation

boneskull
Copy link
Contributor

@boneskull boneskull commented Jul 11, 2023 •
edited

This is a PoC for creating (and shipping) declaration files for LavaMoat packages.

Right now, I've fully typed the sources (not the tests) of @lavamoat/allow-scripts, which now validates the types in "strict" mode. TypeScript is configured for incremental builds (preferable for monorepos), but only builds one module as a PoC.

To generate types, execute npm run build:types in the @lavamoat/allow-scripts workspace. They will be output into packages/allow-scripts/types (which is not under VCS).

I added available type declarations (from DefinitelyTyped), but I had to write a handful of my own for external modules. These do not appear to be re-exported. AFAICT (but I haven't tested it!) there aren't any type declarations that are needed as production dependencies (in @lavamoat/allow-scripts, anyway).

I took some liberties tweaking existing types (narrowing where appropriate, separating @typedefs so that descriptions would be picked up, etc.). I'd prefer if we used the T[] syntax for arrays instead of Array<T>, but that's just my personal preference and I stuck to the current convention.

I note that these .d.ts files I created have semicolons which is a byproduct of my IDE's default formatting. If we intend to keep any of these around, I'd recommend pulling in @typescript-eslint/plugin and its ilk.

@boneskull boneskull force-pushed the boneskull/ts branch 2 times, most recently from 9443213 to eea6bed Compare July 17, 2023 21:23
@boneskull
Copy link
Contributor Author

boneskull commented Jul 24, 2023 •
edited

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@boneskull boneskull changed the base branch from main to boneskull/npm July 27, 2023 20:30
This was referenced Jul 27, 2023
Merged
Merged
Merged
Merged
Merged
@boneskull boneskull force-pushed the boneskull/ts branch 2 times, most recently from f671bab to 21c0e5e Compare July 27, 2023 20:41
@boneskull boneskull marked this pull request as ready for review July 27, 2023 20:56
@boneskull boneskull self-assigned this Jul 27, 2023
@boneskull boneskull changed the title [DRAFT] allow-scripts: prototype of declaration file generation feat(allow-scripts): generate & ship typescript declarations Jul 27, 2023
@boneskull boneskull added the enhancement New feature or request label Jul 27, 2023
@boneskull boneskull force-pushed the boneskull/ts branch 4 times, most recently from 740f172 to ec493f1 Compare July 31, 2023 19:31
@boneskull boneskull mentioned this pull request Jul 31, 2023
Merged
@boneskull boneskull mentioned this pull request Aug 8, 2023
Merged
@boneskull boneskull changed the base branch from boneskull/npm to boneskull/consolidate-eslint August 11, 2023 05:07
This was referenced Aug 15, 2023
Merged
Merged
@boneskull boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from fa30ad5 to ea3095f Compare August 21, 2023 21:45
@boneskull boneskull force-pushed the boneskull/ts branch 2 times, most recently from 171e4fc to 10ddb3a Compare August 22, 2023 21:44
@boneskull boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from 9f11a04 to 5e27065 Compare August 22, 2023 21:50
@boneskull
feat(allow-scripts): prototype of declaration file generation
3c0478d 
This adds `.d.ts` generation to `@lavamoat/allow-scripts`.

- Many type defs added to `src/types` for those modules which do not have them (nor are there any published to DefinitelyTyped).  In the future, some of these could be submitted to DefinitelyTyped, if we wish.
- Upgrade `yargs`
- Create reusable TS configuration supporting incremental builds
- Declaration files generated at build time and shipped in the `types` folder (which is now in `.gitignore`)
- Created `.depcheckrc`
- Added, updated, removed and/or fixed many docstring types

Note: the uncommon `skipLibCheck = false` setting in this module's `tsconfig.json` enables typechecking of the `.d.ts` files themselves.  These files are not compiled.
@boneskull boneskull deleted the branch LavaMoat:boneskull/consolidate-eslint August 22, 2023 23:36
@boneskull boneskull closed this Aug 22, 2023
@legobeat
Copy link
Contributor

I guess this is still relevant and intended to target main after merge of #627 , which seems to have closed this unintentionally?

@boneskull
Copy link
Contributor Author

yeah I think it's because it was targeting a branch that got deleted, it automatically closed, which is not what I expected to happen.

@legobeat
Copy link
Contributor

legobeat commented Aug 22, 2023 •
edited

I would also have expected this PR to automatically retarget. Not sure if GH changed behavior,my memory is incorrect, or if there's some subtlety behind the difference in outcome.

@boneskull boneskull mentioned this pull request Aug 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naugtur naugtur Awaiting requested review from naugtur

@kumavis kumavis Awaiting requested review from kumavis

@leotm leotm Awaiting requested review from leotm

@weizman weizman Awaiting requested review from weizman

@legobeat legobeat Awaiting requested review from legobeat

Assignees

@boneskull boneskull

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@boneskull @legobeat