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

feat: add generated code comments #1037

Merged
merged 9 commits into from
May 1, 2024

Conversation

dasco144
Copy link
Contributor

@dasco144 dasco144 commented Apr 30, 2024

Closes #531

@dasco144 dasco144 force-pushed the add-generated-code-comments branch from 39e13e4 to 71890e4 Compare April 30, 2024 19:40
@dasco144
Copy link
Contributor Author

Looks like the build output gets changed due to the inc;lusion of the package.json as a reference.

image

I assume we'd want to retain the flat structure, so let me see what I can find to fix it.

Comment on lines +330 to +332
const path: string = "../package.json";
const packageJson = await import(path);
const tsProtoVersion = `v${packageJson?.version ?? "unknown"}`;
Copy link
Contributor Author

@dasco144 dasco144 Apr 30, 2024

Choose a reason for hiding this comment

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

This might be a bit of a hack. When providing the path to the dynamic import directly, tsc will include it into the compilation, which leads to the build output having the code in the src sub folder.

By moving it out, and typing it as a string, it can no longer infer that path, and then won't include it in the output.

The path is still valid on the published npm package, from running yarn pack and checking the output.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah cool, good find!

I'm honestly a little surprised the await import just works; do we have to worry about whether ts-proto is invoked in a CJS/ESM environment? I guess this passed the build on node 16, so it must be working.

If users report issues, we could also look at doing a post-tsc step that uses sed/something to replace a hard-coded const tsProtoVersion = "placeholder" type line with the just-being-published version, in the .js file, before upload to npm.

But I'm good with using this and seeing how it goes!

@stephenh stephenh merged commit cdd4a76 into stephenh:main May 1, 2024
6 checks passed
stephenh pushed a commit that referenced this pull request May 1, 2024
# [1.174.0](v1.173.0...v1.174.0) (2024-05-01)

### Features

* add generated code comments ([#1037](#1037)) ([cdd4a76](cdd4a76))
@stephenh
Copy link
Owner

stephenh commented May 1, 2024

🎉 This PR is included in version 1.174.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

bobthecow added a commit to bobthecow/linguist that referenced this pull request Oct 7, 2024
ts-proto [added generated code comments in May](stephenh/ts-proto#1037). This updates linguist to properly detect them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to allow for a generated code comment
2 participants