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

Add checks for TypeScript compliance #8

Closed
desi opened this issue Oct 11, 2023 · 2 comments · Fixed by #55
Closed

Add checks for TypeScript compliance #8

desi opened this issue Oct 11, 2023 · 2 comments · Fixed by #55
Assignees

Comments

@desi
Copy link

desi commented Oct 11, 2023

We want to make sure that for a given project:

  • The project contains a tsconfig.json file, and the content of the file matches the same file in the module template
  • The project contains a tsconfig.build.json file, and the content of the file matches the same file in the module template
  • The project contains a tsup.config.ts file, and the content of the file matches the same file in the module template
  • The project contains @types/node, ts-node, tsup, and typescript as dev dependencies, and the versions match the same dev dependencies as in the module template
  • The project contains the same build and build:types package scripts as in the module template
  • The project's package.json has an exports field, and all of the keys in exports should be present in the project's exports field and the values should match.
  • The project's package.json has a main field, and its value should match the same field in the module template.
  • The project's package.json has a module field, and its value should match the same field in the module template.
  • The project's package.json has a types field, and its value should match the same field in the module template.
  • The project's package.json has a files field, and its value should match the same field in the module template.
  • If project's package.json contains a lavamoat field, and that contains allowScripts, then a tsup>esbuild property should be present and should match the same one in the module template.
@mcmire mcmire transferred this issue from MetaMask/core Oct 12, 2023
@mcmire mcmire changed the title Whether the code is in Typescript or not (Needs to be coverted or not) Add check for whether or not the library has been converted to TypeScript Oct 12, 2023
@mcmire mcmire changed the title Add check for whether or not the library has been converted to TypeScript Add check for TypeScript compliance Oct 12, 2023
@mcmire mcmire changed the title Add check for TypeScript compliance Add checks for TypeScript compliance Oct 12, 2023
@mcmire
Copy link
Collaborator

mcmire commented Jan 18, 2024

Note that there are other TypeScript-related considerations that we could address in this PR but do not need to:

  • The module template contains a lavamoat.allowScripts["tsup>esbuild"] field in its package.json, and ideally all projects should have this too. However, we can only check this if LavaMoat is installed. We don't have any rules around LavaMoat yet, so we can't check this. Added above.
  • There are ESLint packages that are specific to TypeScript, and the lint:eslint package script has the ability to check TypeScript files, and ideally all projects should have both these dependencies along with this package script. However, we can only make these checks if the project uses ESLint. We could literally check the list of dependencies for eslint. At the same time, it seems the rules we're adding in Add checks for whether lint/formatting packages are present and up to date #9 do this for us. So there is a question about how best we ought to do this. We can leave this for another PR, though.
  • The build-lint-test GitHub workflow in the module template builds the project before running tests, using the build package script. We want to make sure that projects have this workflow too, but we can do that in a separate PR (either as a part of Add checks for general module template adherence #6 or some other ticket).
  • At some point, we may consider changing the logic which checks conformance of tsconfig.json and tsconfig.build.json to test for deep containment instead of strict equality. This would allow project maintainers to include extra configuration options without violating the lint rules. We might choose to solve this problem a different way, perhaps via some kind of patching mechanism, however, so we can put that on hold until we figure out a solution.

@desi
Copy link
Author

desi commented Feb 8, 2024

kanthesha added a commit that referenced this issue Mar 1, 2024
<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?

Are there any issues or other links reviewers should consult to
understand this pull request better? For instance:

* Fixes #12345
* See: #67890
-->
We want to make sure that for a given project:

- The project contains a tsconfig.json file, and the content of the file
matches the same file in the module template
- The project contains a tsconfig.build.json file, and the content of
the file matches the same file in the module template
- The project contains a tsup.config.ts file, and the content of the
file matches the same file in the module template
- The project contains @types/node, ts-node, tsup, and typescript as dev
dependencies, and the versions match the same dev dependencies as in the
module template
- The project contains the same build and build:types package scripts as
in the module template
- The project's package.json has an exports field, and all of the keys
in exports should be present in the project's exports field and the
values should match.
- The project's package.json has a main field, and its value should
match the same field in the module template.
- The project's package.json has a module field, and its value should
match the same field in the module template.
- The project's package.json has a types field, and its value should
match the same field in the module template.
- The project's package.json has a files field, and its value should
match the same field in the module template.
If project's package.json contains a lavamoat field, and that contains
allowScripts, then a tsup>esbuild property should be present and should
match the same one in the module template.

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

Successfully merging a pull request may close this issue.

3 participants