charandas commented 5 years ago

765 provided a fix for native modules in Jest context using standard cache.

The repro still does not fully function, now due to a different reason. I have added a commit to the same repo to demonstrate the issue.

I am using Node 10.15.3 LTS.

 FAIL  test/e2e.spec.ts
  Grpc Utils Tests
    ✓ can load proto (24ms)
    ✕ can load proto using the function returned by esm wrapped library (2ms)
    ✓ can call service (25ms)

  ● Grpc Utils Tests › can load proto using the function returned by esm wrapped library

    TypeError: Cannot read property 'readFileSync' of null

      3 |
      4 | export function loadProto (protoFileName, dir) {
    > 5 |   const packageDefinition = protoLoader.loadSync(protoFileName, {
        |                                         ^
      6 |     enums: String,
      7 |     defaults: true,
      8 |     keepCase: true,

      at fetch (node_modules/@grpc/proto-loader/node_modules/protobufjs/src/root.js:160:34)
      at Root.load (node_modules/@grpc/proto-loader/node_modules/protobufjs/src/root.js:194:13)
      at Root.loadSync (node_modules/@grpc/proto-loader/node_modules/protobufjs/src/root.js:235:17)
      at Proxy.loadSync (node_modules/@grpc/proto-loader/build/src/index.js:223:27)
      at Proxy.loadProto (lib/proto.js:5:41)
      at Object.<anonymous> (test/e2e.spec.ts:36:31)

Could you please take a look @jdalton?

charandas commented 5 years ago

@jdalton You may be very busy 😄- I am leaving this place (job changes, lol) where I use esm soon, it would be nice to leave a working copy of that package I wrote, seems I am very close after your previous bugfix.

I tried looking through protobufjs codebase, but its quite dense, and on top of it I don't have the ins on esms internals, otherwise would love to help out. The repro may show something quick to your eyes is my hope.

Take this as a gentle reminder. If it cannot be resolved in the next week, I will drop using that grpc package (esm wrapped) for now, and just repeat myself in each project (no worries).

jdalton commented 5 years ago

Hi @charandas!

Thank you for your patience. I took a 2 week plus vacation (no PC) for the first time in 6 years or so. I'm back now though (slowly getting into things again). I've narrowed down the issue to our custom context object for Jest. This means I'll likely have a patch by this evening : )

Update:

The issue comes down to which eval is being used inside @protobufjs/inquire. For some reason our Jest context isn't providing the expected eval of the realm. I'll need to dig in a bit more to resolve it.

Update:

It looks like a limitation on context objects created by vm.createContext(). I can address it another way but will take some time.

standard-things / esm

protobufjs failing to exercise fs.readFileSync in Jest context #779

765 provided a fix for native modules in Jest context using standard cache.