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

Stubbing Intl makes vi.useFakeTimer crashing #7024

Closed
6 tasks done
dubzzz opened this issue Dec 5, 2024 · 6 comments
Closed
6 tasks done

Stubbing Intl makes vi.useFakeTimer crashing #7024

dubzzz opened this issue Dec 5, 2024 · 6 comments

Comments

@dubzzz
Copy link
Contributor

dubzzz commented Dec 5, 2024

Describe the bug

When stubbing the global Intl and replacing it by a partial copy of it, as follow:

import { test, vi } from "vitest";

const UntouchedIntl = Intl;
test("My test (not working #2)", () => {
  vi.stubGlobal("Intl", { ...UntouchedIntl });
  vi.useFakeTimers();
});

We make vi.useFakeTimer resulting into the error:

TypeError: Cannot read properties of undefined (reading 'prototype')

EDITED: After my first steps of investigations, I'd say that if there is a bug on your side, it's limited to the fact that timers rely on potentially altered instances of Intl (and also others). If it's done on purpose just close my issue as the problem is mostly user side (see my suggestions of fixes on userland at #7024 (comment)).

Reproduction

I put multiple reproductions in the repository: https://github.com/dubzzz/bug-vitest-intl. By iterating over my initial test case that was aiming into stubbing Intl.NumberFormat to make sure it always relies on the US local, I reached several sub-cases with some working and others not working.

Something pretty interesting is that the stub is only causing troubles if done before the call to vi.useFakeTimer.

This one crashes:

import { test, vi } from "vitest";

const UntouchedIntl = Intl;
test("My test (not working #2)", () => {
  vi.stubGlobal("Intl", { ...UntouchedIntl });
  vi.useFakeTimers();
});

This one does not crash:

import { test, vi } from "vitest";

const UntouchedIntl = Intl;
test("My test (working #1)", () => {
  vi.useFakeTimers();
  vi.stubGlobal("Intl", { ...UntouchedIntl });
});

This one does not crash:

import { test, vi } from "vitest";

const UntouchedIntl = Intl;
test("My test (working #2)", () => {
  vi.stubGlobal("Intl", UntouchedIntl);
  vi.useFakeTimers();
});

Please note that my initial attempt (I'm gonna add it into the repo too) was only trying to spy on Intl.NumberFormat but it actually made the produced formatters fail when called on format in some cases. But so far I've not been able to restrict this case to something smaller that I can share.

System Info

System:
    OS: Linux 6.5 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (2) x64 Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
    Memory: 5.49 GB / 7.74 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 20.17.0 - ~/nvm/current/bin/node
    Yarn: 1.22.22 - /usr/bin/yarn
    npm: 10.8.2 - ~/nvm/current/bin/npm
    pnpm: 9.11.0 - ~/nvm/current/bin/pnpm
  npmPackages:
    vitest: ^2.1.8 => 2.1.8

Used Package Manager

npm

Validations

@dubzzz
Copy link
Contributor Author

dubzzz commented Dec 5, 2024

I found this trick to patch the issue:

++ const SafeIntl = Intl;
   function requireFakeTimersSrc () {
   //...
   	function withGlobal(_global) {
   //...
	       const cancelIdleCallbackPresent =
	           _global.cancelIdleCallback &&
	           typeof _global.cancelIdleCallback === "function";
	       const setImmediatePresent =
	           _global.setImmediate && typeof _global.setImmediate === "function";
--	       const intlPresent = _global.Intl && typeof _global.Intl === "object";
++	       const intlPresent = SafeIntl && typeof SafeIntl === "object";

But I don't know if it would be acceptable for you. Maybe the use of the last instance of Intl is done on purpose.

@dubzzz
Copy link
Contributor Author

dubzzz commented Dec 5, 2024

The crash seems to happen here:

	        ClockIntl.DateTimeFormat.prototype = Object.create(
	            NativeIntl.DateTimeFormat.prototype // undefined
	        );

@dubzzz
Copy link
Contributor Author

dubzzz commented Dec 5, 2024

What do you think about the idea of making sure that we use the untouched global and not one that may have been touched before we access it? We could rewrite SafeIntl that way const SafeIntl = typeof Intl !== 'undefined' ? Intl : undefined and then only rely on this instance.

If acceptable I can work on providing such fix, if you are ok with the idea.

@dubzzz
Copy link
Contributor Author

dubzzz commented Dec 5, 2024

In case other people face the same issue in the future, I have two working workarounds.

Option 1

We just make the stub a little bit more complex but we only rely on the primitives offered by Vitest.

import { test, vi } from "vitest";

const UntouchedIntl = Intl;
const UntouchedIntlDescriptors = Object.getOwnPropertyDescriptors(Intl);

test("My test (working #4)", () => {
  const defaultLocale = "en-US";
  const NewIntl = {};
  Object.defineProperties(NewIntl, {
    ...UntouchedIntlDescriptors,
    NumberFormat: {
      ...UntouchedIntlDescriptors.NumberFormat,
      value: class extends UntouchedIntl.NumberFormat {
        constructor(...args) {
          if (args.length < 1) {
            super(defaultLocale);
          } else {
            const [locales, ...otherParams] = args;
            if (
              typeof locales === "string" ||
              (Array.isArray(locales) && locales.length !== 0)
            ) {
              super(...args);
            } else {
              super(defaultLocale, ...otherParams);
            }
          }
        }
      },
    },
  });
  vi.stubGlobal("Intl", NewIntl);
  vi.useFakeTimers();
});
Option 2

We bypass Vitest and pollute the prototype in a global way.

// in vitest.setup.js

const defaultLocale = "en-US";
Intl.NumberFormat = class extends Intl.NumberFormat {
  constructor(...args) {
    if (args.length < 1) {
      super(defaultLocale);
    } else {
      const [locales, ...otherParams] = args;
      if (
        typeof locales === "string" ||
        (Array.isArray(locales) && locales.length !== 0)
      ) {
        super(...args);
      } else {
        super(defaultLocale, ...otherParams);
      }
    }
  }
};

@sheremet-va
Copy link
Member

What do you think about the idea of making sure that we use the untouched global and not one that may have been touched before we access it? We could rewrite SafeIntl that way const SafeIntl = typeof Intl !== 'undefined' ? Intl : undefined and then only rely on this instance.

If acceptable I can work on providing such fix, if you are ok with the idea.

I think if anything, this should be done on sinon side. I don't think this is an issue with Vitest, the stubGlobal just does this internally:

globalThis[key] = value

@dubzzz
Copy link
Contributor Author

dubzzz commented Dec 5, 2024

Closing it then. I'll open a follow up on sinon side. Thanks for your quick answer.

@dubzzz dubzzz closed this as completed Dec 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants