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

As implemented, module-scoped constants may not work properly #113

Open
popham opened this issue Dec 20, 2018 · 4 comments
Open

As implemented, module-scoped constants may not work properly #113

popham opened this issue Dec 20, 2018 · 4 comments

Comments

@popham
Copy link
Contributor

popham commented Dec 20, 2018

I noticed that your schema's constants get translated to naked values, e.g. capnpc-ts/test/integration/test.capnp.ts#L2110-L2145. Unfortunately, ES6 and Node's module resolution don't guarantee that imports are available at module scope. When you go to wrap an imported struct around some data for a module-scoped constant, you may discover that the imported struct is not yet available. Translating someConstant to getSomeConstant() generalizes to module scope without any problems, but I'm curious if you've got some other workaround.

@jdiaz5513
Copy link
Owner

jdiaz5513 commented Dec 20, 2018 via email

@popham
Copy link
Contributor Author

popham commented Dec 20, 2018

The problem will pop up in generated code. The Cap'n Proto schema language admits circular imports, so your generated code will encounter circular imports like the following:

// file1.capnp
const c1 :import "file2.capnp".F2 = ();
struct F1 {}
// file2.capnp
const c2 :import "file1.capnp".F1 = ();
struct F2 {}

If you're exposing value assignments as scoped constants, then a constant at a schema's file scope translates to an assignment at module scope, e.g.

//file1.capnp.ts
import { F2 } from "./file2.capnp";
export const C_1 = new F2...;
//file2.capnp.ts
import { F1 } from "./file1.capnp";
export const C_2 = new F1...;

When the module initializes, the F1 and F2 are not necessarily available for new.

I should monkey around with the code anyway; I'll submit a PR that amends packages/capnpc-ts/test/integration/import-bar.capnp and packages/capnpc-ts/test/integration/import-foo.capnp to demonstrate.

@popham
Copy link
Contributor Author

popham commented Dec 21, 2018

Your struct-typed constants currently call capnp.rawPointer to yield a Pointer instance, so circular imports will work fine as currently implemented. If you intend to ever construct the actual struct types, however, then my proposed problem will arise. I was only able to look at struct-scoped constants because file-scoped constants did not get translated.

@popham
Copy link
Contributor Author

popham commented Dec 21, 2018

I've added a hypothetical export of the module-scoped constant to #116. Running tests as written clobbered the hand coded additions, so I've altered gulp's test:capnpc-js task to stop the task from overwriting the hand-coded additions. Running gulp test:capnpc-js, note the error: import _foo_capnp_1.Foo is not a constructor. It chokes on the constructor before it even tries to work with a (null as any). Toss in a if (Foo===undefined) { throw new Error("PEEP"); } just before the const FOO =... line, and you'll get a "PEEP".

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

No branches or pull requests

2 participants