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

js/{closure,commonjs}: create a new js_module option #65

Open
glerchundi opened this issue Sep 7, 2018 · 18 comments
Open

js/{closure,commonjs}: create a new js_module option #65

glerchundi opened this issue Sep 7, 2018 · 18 comments
Labels
enhancement New feature or request help wanted Extra attention is needed javascript triaged Issue has been triaged

Comments

@glerchundi
Copy link

glerchundi commented Sep 7, 2018

What language does this apply to?

If it's a proto syntax change, is it for proto2 or proto3? proto3
If it's about generated code change, what programming language? javascript/commonjs but it can be applied to es6 or closure as well.

Describe the problem you are trying to solve.

We are generating code of multiple packages that we want to publish independently but with references between them. Currently the generator creates a tightly coupled relative paths between folders.

Describe the solution you'd like

An option like Go's go_package or C#'s csharp_namespace where I can put the import path, in this case the JS module.

I think a js_module called option would do the trick.

Describe alternatives you've considered

Overriding the generated code.

@glerchundi glerchundi changed the title js/commonjs: create a new js_module option js/{closure,commonjs}: create a new js_module option Sep 7, 2018
@xfxyjwf xfxyjwf added enhancement New feature or request javascript labels Sep 7, 2018
@xfxyjwf xfxyjwf self-assigned this Sep 7, 2018
@glerchundi
Copy link
Author

@xfxyjwf friendly ping!

@aberasarte
Copy link

Any update on this ?

@glerchundi
Copy link
Author

Another friendly ping to @anandolee @TeBoring @BSBandme!

@glerchundi
Copy link
Author

This is related to protocolbuffers/protobuf#2198, is it possible any maintainer give us a status update or how they think this can be solved/workaround?

We want to start collaborating with envoyproxy/lyft and its protoc-gen-validate and this would open up the possibility to make it happen.

@BSBandme BSBandme assigned TeBoring and BSBandme and unassigned BSBandme and TeBoring Jun 11, 2019
@BSBandme
Copy link

In fact internally we have some options to tweak the namespace. But we still don't have a way for commonjs. There're some legacy reasons that we didn't open source this (we have several different js protobuf implementations internally).

Will discuss about this in next team meeting.

@glerchundi
Copy link
Author

In fact internally we have some options to tweak the namespace. But we still don't have a way for commonjs. There're some legacy reasons that we didn't open source this (we have several different js protobuf implementations internally).

Thanks for the update and the details.

Will discuss about this in next team meeting.

Did that meeting already occurred, and it case it did do you have a path forward for this issue?

Thanks.

@glerchundi
Copy link
Author

Believe me that I feel really really bad for pinging you all again but we're really interested in working on this and would love to see at least a status update and/or roadmap for a possible publishing or reimplementation on upstream. 🙏

@4nte
Copy link

4nte commented Mar 12, 2020

@BSBandme, can you tell us if there are any plans to implement this or suggest a workaround, I'd greatly appreciate it!

This is blocking for me, I'm planning to work around this by patching the generated code to remove relative imports with corresponding npm packages.
That is, replace require("../sibling/sibling.proto") with require("npmlib/sibling.proto").

I'm trying to decouple proto packages into separate npm modules, and have them depend on each other.

@aberasarte
Copy link

That is, replace require("../sibling/sibling.proto") with require("npmlib/sibling.proto").

This is exactly what we are doing. We have a shell script that replaces the relative imports with package imports. It's ugly and error prone but is the only workaround we have at the moment.

It's been a while since the last update, I would love to hear if there are any plans to add this feature in the near future.

@aberasarte
Copy link

@BSBandme could you shed some light on this matter please?

Our building script is now a little monster that is really hard to maintain and is breaking every time we add new dependencies.

This feature would make things much easier for us.

Thanks in advance

@buzzdan
Copy link

buzzdan commented Dec 2, 2020

Watching this one closly
We also work with protos and node and would love to have validations with lyft's PGV project. (We work with it already in golang, clojure & python).

@buzzdan
Copy link

buzzdan commented Jan 13, 2021

friendly ping to @anandolee @TeBoring @BSBandme 🙏

@glerchundi
Copy link
Author

Also another friendly reminder for @perezd as he seems to be the one driving javascript related issues. 🙏

@perezd
Copy link

perezd commented Jan 13, 2021

Howdy, thanks for reaching out. Addressing this is currently not on our roadmap at this time. We're in the process of revamping our comprehensive Protobuf support for JavaScript/TypeScript and would ask that you remain patient with us as we attempt to address the larger needs of the project both internally and externally. I hope you understand the position we're in with our resources and responsibilities.

@glerchundi
Copy link
Author

@perezd to be honest this is taking more time than I thought, even for triaging! 😭

Without being too dramatic and trying to set real expectations, do you think this will be addressed in the next, for instance, 6 months? I really wanna go through the standard path but if there is no an expected timeline for this I will try to think & implement alternative ways.

Thanks!

@perezd
Copy link

perezd commented Aug 24, 2021

heya, our roadmap/objectives have shifted and I do not believe you'll see this in that timeframe. sorry :(

@apssouza22
Copy link

While we don't have this sorted, I am handling this issue with a simple bash function https://github.com/apssouza22/modern-api-management/blob/master/bin/js-gen.sh#L105

@acozzette acozzette transferred this issue from protocolbuffers/protobuf May 16, 2022
@dibenede dibenede added triaged Issue has been triaged help wanted Extra attention is needed labels Sep 23, 2022
@dibenede
Copy link
Contributor

We understand this is a problem, but are unclear what the best solution would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed javascript triaged Issue has been triaged
Projects
None yet
Development

No branches or pull requests

10 participants