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

fix: improve package.json lookup for root name #1530

Merged
merged 2 commits into from
Mar 23, 2024

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Dec 15, 2023

compileProtos seeks for package.json one directory above the one it accepts as input (typically src).

Running gapic-generator-typescript with --format=esm, generates the the sources in esm/src, then compileProtos can't find package.json.

When package.json is not found, the root name falls back to default and all the packages have the same root.

Use walk-up-path, which is also used by npm[1].

Fixes #1529.

[1] https://github.com/npm/config/blob/77a48dbe22/lib/index.js#L632

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@orgads orgads requested review from a team as code owners December 15, 2023 08:23
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Dec 15, 2023
@orgads
Copy link
Contributor Author

orgads commented Jan 20, 2024

Rebased.

tools/src/compileProtos.ts Show resolved Hide resolved
@orgads orgads force-pushed the package-json-lookup branch 2 times, most recently from 7173d84 to 13576ae Compare February 4, 2024 18:51
@orgads
Copy link
Contributor Author

orgads commented Feb 4, 2024

@sofisl Please review.

compileProtos seeks for package.json one directory above the one it
accepts as input (typically src).

Running gapic-generator-typescript with --format=esm, generates the
the sources in esm/src, then compileProtos can't find package.json.

When package.json is not found, the root name falls back to default
and all the packages have the same root.

Use walk-up-path, which is also used by npm[1].

Fixes googleapis#1529.

[1] https://github.com/npm/config/blob/77a48dbe22/lib/index.js#L632
@orgads
Copy link
Contributor Author

orgads commented Feb 19, 2024

Rebased. @sofisl ping.

@sofisl sofisl self-requested a review March 23, 2024 02:15
@sofisl sofisl added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 23, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 23, 2024
@sofisl sofisl added automerge: exact Summon MOG for automerging, but approvals need to be against the latest commit owlbot:run Add this label to trigger the Owlbot post processor. labels Mar 23, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 23, 2024
@sofisl sofisl merged commit 3ec09cf into googleapis:main Mar 23, 2024
18 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge: exact Summon MOG for automerging, but approvals need to be against the latest commit label Mar 23, 2024
@release-please release-please bot mentioned this pull request Mar 23, 2024
@orgads orgads deleted the package-json-lookup branch March 23, 2024 18:24
leahecole pushed a commit to leahecole/gax-nodejs that referenced this pull request Mar 27, 2024
compileProtos seeks for package.json one directory above the one it
accepts as input (typically src).

Running gapic-generator-typescript with --format=esm, generates the
the sources in esm/src, then compileProtos can't find package.json.

When package.json is not found, the root name falls back to default
and all the packages have the same root.

Use walk-up-path, which is also used by npm[1].

Fixes googleapis#1529.

[1] https://github.com/npm/config/blob/77a48dbe22/lib/index.js#L632

Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compileProtos: Generating for esm/src uses default root
3 participants