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

pbjs/sparse: fix namespace replacement, include transitive dependencies #1528

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

Conversation

imirkin
Copy link

@imirkin imirkin commented Dec 30, 2020

This fixes sparse mode for my use-case. Protos are spread over several
files, and may include nested messages/enums from other files.
Additionally, this fixes the case where a Type is replaced by a Namespace,
e.g. when an enum is used without its containing message type.

Fixes #1436

@alexander-fenster
Copy link
Contributor

This looks reasonable, we'll just need some more time to review the change and we'll get back to you @imirkin. Thanks for the PR!

This fixes sparse mode for my use-case. Protos are spread over several
files, and may include nested messages/enums from other files.
Additionally, this fixes the case where a Type is replaced by a Namespace,
e.g. when an enum is used without its containing message type.

Fixes protobufjs#1436
@binsee
Copy link
Contributor

binsee commented Sep 23, 2022

Is there any progress on this please? @alexander-fenster

@imirkin
Copy link
Author

imirkin commented Jun 2, 2023

Is there any chance this will ever get in? Should I just close it? Rebase it?

I can't use --sparse without this patch, since the current logic ends up over-removing types which are actually in use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sparse fails when referencing a nested enum
3 participants