Support Long instead of throwing error

[stephenh]

Support Long instead of throwing error

[stephenh]

Hi @gnzzz yep, I think a plugin option is a good idea. There is an option already called context that shows how some of that works. Are you up for submitting a PR around that? I could give a few hints but in theory it shouldn't be too bad..

[gnzzz]

I got an initial stab working for the option at https://github.com/gnzzz/ts-proto . It feels like I'm butchering the ts-poet statements though, so any pointers there would be helpful.

The output looks pretty good at the moment, and I will write a simple test in the same way as the other. If there are any other scenarios that I should test, please let me know.

[stephenh]

@gnzzz this looked really good! I went ahead and merged it.

I was seeing some test/compile errors; can you check out this commit: 5706ff9

...what's the thing with import * as Long from "long" vs. import Long from "long"? I was pretty sure one of the tsconfig values, like esModuleInterop or allowSyntheticDefaultImports allowed always using the 2nd version...

Should we change the code generated output to use import Long from "long"? For that commit above, I just did it by hand to see if it'd work / get the tests to pass.

[gnzzz]

...what's the thing with import * as Long from "long" vs. import Long from "long"? I was pretty sure one of the tsconfig values, like esModuleInterop or allowSyntheticDefaultImports allowed always using the 2nd version...

Yes, you're right. esModuleInterop set to true( or allowSyntheticDefaultImports depending on how you see it ) allows you to use import Long from "long" for any commonjs package. My understanding, which might be wrong, is that import * will be a namespace import. That is to say if it's a class like structure, using that is safe on any legacy setting of typescript as long as you treat it as a class. import Long from "long" is a default import and will only work if the package has .default export. This is where esModuleInterop comes in as the compiler will automatically convert any import that does not have a default to use the top level export as default.

The problem here is that a package normally has the luxury to decide this internally, but ts-proto is a bit special as it ejects code that will be used by other projects. Since esModuleInterop was not true by default a couple of years ago then this will force developers to change.

Interestingly, one of our projects doesn't have this setting and with the import Long from "long" the compiler will error with TS1192: Module node_modules/@types/long/index has no default export.

Was the error in CircleCI? If that's the case, I think that leaving it as import * and then using Long.fromNumber as you did or Long.ZERO should work?

[gnzzz]

I can experiment a bit with this tomorrow if you can enable CircleCI for PRs or forks. I don't have any experience with Circle but seems like a small configuration change would allow it.

[stephenh]

@gnzzz ah nice, that was a great explanation, particularly that downstream projects may/may not have esModuleInterop set. I've pushed a commit that removes esModuleInterop from the tsconfig.json and changes back to use import * for both long as well as dataloader and object-hash, which whether other packages I was cheating a bit on and leaning on the esModuleInterop flag.

I believe everything should be good now?

Take a look at the last few commits and let me know, and then I'll push out a new release.

I also enabled the "build forks" setting in CircleCI, afaiu...

Thanks!

[gnzzz]

@stephenh that looks great, just tried the master version and the output works perfectly. Thank you very much.

I'll put up a new PR with the documentation you requested shortly.

[stephenh]

Great, I'll go ahead and close this out. Thanks again for the contribution!

[stephenh]

I released v1.9.0 with your commits.