Skip to content

docs: generate adev-compatible api json #30857

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mmalerba
Copy link
Contributor

Currently using copied in script from adev, need to move the script to a shared location

@mmalerba mmalerba force-pushed the cdk-api branch 4 times, most recently from 0c6a191 to 50e19f9 Compare April 21, 2025 21:25
@angular-robot angular-robot bot added the area: docs Related to the documentation label Apr 21, 2025
@mmalerba mmalerba changed the title WIP: generate adev-compatible api json docs: generate adev-compatible api json Apr 21, 2025
@mmalerba mmalerba marked this pull request as ready for review April 21, 2025 21:26
@mmalerba mmalerba requested review from a team as code owners April 21, 2025 21:26
@mmalerba mmalerba requested review from crisbeto, wagnermaciel and andrewseguin and removed request for a team, crisbeto and wagnermaciel April 21, 2025 21:26
@mmalerba mmalerba added the target: patch This PR is targeted for the next patch release label Apr 21, 2025
@mmalerba mmalerba force-pushed the cdk-api branch 2 times, most recently from dc03d7b to de4f33a Compare April 21, 2025 21:52
@josephperrott josephperrott force-pushed the cdk-api branch 3 times, most recently from a0aa103 to 82f26cf Compare April 25, 2025 20:14
@mmalerba mmalerba removed the request for review from a team April 28, 2025 14:54
@@ -0,0 +1,70 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use js_binary? nodejs_binary is the old one and doesn't support ESM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a try, and it tells me that I have to run the js_binary with js_run_binary and that seems to be a whole can of worms that involves converting extract_api_to_json from a rule to a macro. And that probably entails changing the format of the parameters passed into the actual script.

This is all just copy/pasted from the fw repo with the idea that I would make minimal changes to it and it would eventually be refactored to just call the same script from some shared location.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to just call the js_binary like the nodejs_binary. You just need to set BAZEL_BINDIR as environment variable.

I don't think, even if it's for copy/pasting, that we should introduce new instances of legacy code that we were removing/replacing in the last weeks/month.

# Pass the import path map
# TODO: consider module_mappings_aspect to deal with path mappings instead of manually
# specifying them
# https://github.com/bazelbuild/rules_nodejs/blob/5.x/internal/linker/link_node_modules.bzl#L236
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: update this TODO. With our new Bazel toolchain, this whole concept no longer exists (probably module_name attributes still need to be cleaned up though).

This way seems correct, but even better would be just leveraging the real tsconfig from src/bazel-tsconfig-build with the "real path mappings".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Related to the documentation target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants