Cannot assign 'NaN' of object '#<Window>'

[mnowik]

I just bumped your package from version 0.0.11 -> 0.1.1 and I am getting the following error.

  ● Test suite failed to run

    TypeError: Cannot assign to read only property 'NaN' of object '#<Window>'
        at Function.assign (<anonymous>)

      10 | });
      11 |
    > 12 | Object.assign(
         |        ^
      13 |   global,
      14 |   new (cw)(
      15 |     fs.readFileSync(path.join(__dirname, 'index.js'), 'utf-8'),

      at Object.assign (setupTests.js:12:8)
          at Array.forEach (<anonymous>)

As mentioned in the CF post, here is my test setup to mock fetch.

Object.assign(
  global,
  new (cw)(
    fs.readFileSync(path.join(__dirname, 'index.js'), 'utf-8'),
    {
      bindings: { fetch: jestFetch }
    },
  ).context
);

[hankjacobs]

How are you running these tests? With Jest? I'm a little confused as to why global is an instance of Window (as is mentioned in the error message). Also, can you please link to the CF post? I'm not sure that I've seen it.

Either way, in v0.1.1 I added the injection of a bunch of Javascript globals into the context that ultimately runs the worker script. This was done to allow the use of instanceof within a worker. This is necessary because of the way cloudworker is architected and the way realms within Node VMs work.

Note in the snippet how you are assigning the .context property of an instance of Cloudworker to the global object. Rather than using the whole context, I'd assign only the specific properties of the context that are necessary and that depends on what is used within your tests.

[mnowik]

Yes, I am using Jest.
Here is the blog post: https://blog.cloudflare.com/unit-testing-worker-functions/

[hankjacobs]

Ah I see. Yeah, my advice in my previous comment still stands. Assigning all of the context properties to global is way too broad. You should assign only what is used. Use something like the following instead (this is untested code so it may not work without minor tweaks):

context = new (require('@dollarshaveclub/cloudworker'))(require('fs').readFileSync('worker.js', ‘utf8’)).context // taken from the blog post
global.Request = context.Request
global.URL = context.URL
(rinse and repeat for every required property)

Alternatively, you could delete the NaN property that's causing the TypeError. Note that you may have to do it for other properties. I'm not sure which ones Jest freezes.

context = new (require('@dollarshaveclub/cloudworker'))(require('fs')readFileSync(path.join(__dirname, 'index.js'), 'utf-8')).context
delete context.NaN
Object.assign(
  global,
  context
)

There's probably a handful of other ways as well. If I had to vote, I'd go with my first suggestion. Blindly assigning properties from one object to another is a recipe for breakage (as you see here). Let me know what you use.

[mnowik]

Thanks, @hankjacobs

Ended up implementing your solution, it is definilty more explicite than assigning everything.

context = new (require('@dollarshaveclub/cloudworker'))(require('fs').readFileSync('index.js', 'utf8')).context;

// Overwrite context
context.fetch = jestFetch;

// Global setup
...
global.fetch = context.fetch;
global.atob = atob;