Rules_nodejs: [feat] auto add @npm//@types/pkg if it exists when @npm//pkg is in deps

[Toxicable]

🚀 feature request

Relevant Rules

ts_library

Description

A simple addition that automatically resolves the types package for those external packages that don't come with types themselves.

Describe the solution you'd like

When a package is in the deps array, for example: @npm//express
The user should not have to define @npm//@types/express - ts_library should add it for them.
If the types package does not exist, then it should not fail and allow the TS compiler to fail later on if the user has noImplicitAny turned on - for packages that don't ship with types.

Describe alternatives you've considered

Manually adding the typing packages

[santoshyadavdev]

Hi @alexeagle ,
Can I work on this, may need some help.

[Toxicable]

@santoshyadav198613 You're welcome to give this a shot if you like

[santoshyadavdev]

thanks will try

[santoshyadavdev]

Hey Guys, build_packages_all.sh do I need to make changes to this file. Or can you guide me to where I have to make changes?

[Toxicable]

build_packages_all.sh is not the right place, that dosen't get called when you use this repo downstream.
However, if you edited generate_build_file.js so that the filegroup's have a dependancy on their types (if it exists), that might do the trick

[qzmfranklin]

Indeed, we have done something similar to this already. But the code relies on some hard coded rules specific to our own repo. Would love to see this motivation moving forward.

[santoshyadavdev]

Hi @Toxicable ,
One question here I found the code, the question here is we extract the packages from package.json, and now to add type definition, we have to check from the registry is type definition exists.

Is it the correct way, or I am missing something

[Toxicable]

@santoshyadav198613 no need to check the registry, you should only check if there's a types package for it installed in th ecurrent package.json.

For example:
Processing package lodash

1. we create @npm//lodash
2. we check if the @types/lodash exists in the package.json
3. if it exists then we check if @npm//@types/lodash exists and create it if not
4. once @npm//@types/lodash is there we add it as a dep to @types/lodash

[santoshyadavdev]

HI @Toxicable ,
I was looking for a guide on how to use the build locally, can you share.
I have made the changes, but want to verify before I open for review.

[santoshyadavdev]

Hi @Toxicable,
Sorry for the delay here, I have processed express package and added before and after BUILD.bazel file here, let me know if this is what we are expecting.

BUILD.zip

[Toxicable]

For this change we need to make the changes I described above to generate_build_file.js for any npm package, not just one specific one.
Further than that we need to commit those changes, uploading a zip here of the processed file does not solve this issue.

[santoshyadavdev]

Sorry if I was not clear, I just wanted to ask if this is the expected output.

[Toxicable]

@santoshyadav198613 Gotcha, in that case why not put up a PR so we can check it in CI
That's a better verification than checking one file

[santoshyadavdev]

Hi @Toxicable ,

I have pushed the code, may be incorrect, But tried my best as per what I understood.
Thanks a lot for your help.

[Toxicable]

After relooking at this I din't think we should go ahead with this.
It would introduce a breaking change - users would have to remove current references to @npm//@types/pkg targets.

The core problem should be solved by a build file generation program. https://github.com/bazelbuild/rules_nodejs/issues/789

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

[github-actions[bot]]

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"