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

build: fix TypeScript prepack command #38

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

aloisklink
Copy link
Collaborator

Fix the TypeScript npm run generate_types command by:

  • Only deleting files in the src/ dir, not in the node_modules dir
  • Skipping building the types from .test.js files
  • Fixing some type errors caused by e03371f

I've also reverted the TypeScript ES2020 version that was used before e03371f, since we're not using any ES2022 features, and if we do start using them, I'm worried it might be a breaking change for users.

Fix the TypeScript `npm run generate_types` command by:

- Only deleting files in the `src/` dir, not in the `node_modules` dir
- Skipping building the types from `.test.js` files
- Fixing some type errors caused by the Lodash replacement PR
@tbo47
Copy link
Owner

tbo47 commented Sep 23, 2024

Thank you @aloisklink

Currently the project is not publishing types. So I think people are relying on these projects for types:
https://www.npmjs.com/package/@types/dagre
https://www.npmjs.com/package/@types/dagre-d3
https://www.npmjs.com/package/@types/graphlib

When I tried to migrate to typescript, we realized that there is minor discrepancies between what is generated by the code and these projects. See #28

I'm worried publishing the types is going to have side effects for people. Thoughts?

Maybe we can publish the types ni different projects, like @types/dagre-es or something similar.

@aloisklink
Copy link
Collaborator Author

Currently the project is not publishing types.

I think we are publishing the auto-generated types from the .js files (although very very very basic ones).

I had a look at https://arethetypeswrong.github.io/, and every single version of dagre-d3-es since version 5.0.1 seems to have types.

image

If we remove them, that would cause side-effects for people.

When I tried to migrate to typescript, we realized that there is minor discrepancies between what is generated by the code and these projects. See #28

😢 I'm guessing that's because most of this code is written in a very very old style of JavaScript, that nobody uses nowadays.

As a temporary measure, we can use JSDoc comments in .js files to make TypeScript happy, which is what I have done in this PR. It's not ideal, but it works and since everything is a comment, it doesn't affect the output .js.

@tbo47 tbo47 merged commit 261104e into tbo47:main Sep 23, 2024
2 checks passed
@aloisklink aloisklink deleted the build/fix-typescript branch September 23, 2024 12:16
@aloisklink aloisklink mentioned this pull request Sep 23, 2024
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.

2 participants