Remove references to window

[Emilcrafter]

• Remove references to window in node-web-compat-web.ts This is a fix for the next.js edge runtime

Issue #, if available: 108

Description of changes: As suggested in the issue thread, references to window were removed. window.setTimeout.bind(window) was replaced with setTimeout.bind(undefined). This fixed the errors that were previously experienced. I suggest to add one or more tests that cover support for next.js edge runtime, since this package is an amazing fit for middleware.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[ottokruse]

LGTM

I suggest to add one or more tests that cover support for next.js edge runtime

Do you have a suggestion? How can we use the Next edge runtime in tests? It would be nice. OTOH it's not a blocker to me. What do you think @hakanson ?

[ottokruse]

And THANKS for the PR :)

[Emilcrafter]

You could create a barebones next.js project with authentication middleware and run Cypress tests like with the other frameworks. The server that runs locally on my computer with next start / next dev acts the same way as the production environment on Vercel when it comes to middleware (in this particular situation at least). That being said, I am not an experienced tester by any means. Thank you!

[hakanson]

Note: I had added window in #80 because of this error

"Illegal invocation" error happening in SimplePenaltyBox.registerFailedAttempt in browser, likely caused by setTimeout binding - updated to window.setTimeout.bind(window)

[ottokruse]

Note: I had added window in #80 because of this error

"Illegal invocation" error happening in SimplePenaltyBox.registerFailedAttempt in browser, likely caused by setTimeout binding - updated to window.setTimeout.bind(window)

Ok, I think we are covered for that as setTimeout.bind(undefined) will have the same end result in the browser. Before we didn't do the .bind() which led to an error as the function was called like this nodeWebCompat.setTimeoutUnref(...) which led to nodeWebCompat becoming the this argument, which was not right. Having undefined be the this argument is fine, because that is the same (in 'strict mode') as when you just call setTimeout directly (without leading nodeWebCompat.).

[ottokruse]

You could create a barebones next.js project with authentication middleware and run Cypress tests like with the other frameworks. The server that runs locally on my computer with next start / next dev acts the same way as the production environment on Vercel when it comes to middleware (in this particular situation at least). That being said, I am not an experienced tester by any means. Thank you!

Ok sounds doable, but let's proceed without this for now. Instead would you mind to add an eslint rule like this to our eslint config in this PR @Emilcrafter ? Then we won't forget this lesson we now learned in the future:

"no-restricted-globals": [
      "error",
      {
        name: "window",
        message:
          "Don't use the window global, as you don't need to use it and it's not a defined global in the Next.js Edge Runtime",
      },
    ],

[Emilcrafter]

Eslint rule was added

[ottokruse]

🎉 Thanks @Emilcrafter