Use the built-in AggregateError when possible

[aleclarson]

If globalThis.AggregateError exists, use that instead of our polyfill. Alternatively, remove AggregateError altogether and expect the end user to polyfill it themselves.

Related issue

sodiray/radash#392

[MarlonPassos-git]

I will work on it.

[MarlonPassos-git]

Hi @aleclarson

This issue can be easily resolved with a small modification to the file:

class AggregateErrorPolyfill extends Error {
  errors: Error[]
  constructor(errors: Error[] = []) {
    super()
    const name = errors.find(e => e.name)?.name ?? ''
    this.name = `AggregateError(${name}...)`
    this.message = `AggregateError with ${errors.length} errors`
    this.stack = errors.find(e => e.stack)?.stack ?? this.stack
    this.errors = errors
  }
}

export const AggregateError = globalThis?.["AggregateError"] ?? AggregateErrorPolyfill

However, this can be a bit more complicated due to the current tsconfig configuration, where globalThis might not have the AggregateError type definitions.

We could resolve this in a couple of ways:

1. Using a workaround:

   // @ts-ignore
   export const AggregateError: typeof AggregateErrorPolyfill = globalThis?.["AggregateError"] ?? AggregateErrorPolyfill

2. Modifying the tsconfig:

   Add "ES2021.Promise" to the lib configuration:

   "lib": ["es2020", "ES2021.Promise"],

   Then define AggregateError with the appropriate type:

   export const AggregateError: AggregateErrorConstructor = globalThis?.["AggregateError"] ?? AggregateErrorPolyfill

Regarding tests:

The current tests check the implementation of AggregateErrorPolyfill, which might differ from how the native AggregateError works.

With the current way we import the function in our tests, we will end up importing the native implementation.

However, it is possible to simulate an environment where AggregateError is not available and force the import of the polyfill version with:

vi.stubGlobal('AggregateError', undefined)
const { AggregateError } = await import('radashi')

But this would cause another type error:

In this case, we would need to either use @ts-ignore or modify the tsconfig to accommodate the test files.

That being said, it would be useful to add two more tests to ensure that when globalThis.AggregateError is undefined, our polyfill is used, and when it exists, the value of our stub is correctly returned.

What are your thoughts on this?

[aleclarson]

Add "ES2021.Promise" to the lib configuration:

Let's put /// <reference lib="es2021.promise" /> at the top of AggregateError module so we can keep tabs on why something beyond es2020 is needed.

export const AggregateError: AggregateErrorConstructor = globalThis?.["AggregateError"] ?? AggregateErrorPolyfill

I think it's fine to assume globalThis exists. The new pnpm lint:compat command doesn't complain.

Also, let's use typeof globalThis.AggregateError as the type and inline the class declaration like so:

export const AggregateError: typeof globalThis.AggregateError =
  globalThis.AggregateError ??
  class AggregateError {…}

With the current way we import the function in our tests, we will end up importing the native implementation.

However, it is possible to simulate an environment where AggregateError is not available and force the import of the polyfill version with:

vi.stubGlobal('AggregateError', undefined)
const { AggregateError } = await import('radashi')

I will take care of this part. Let me know when you open the PR 👍

[aleclarson]

Fixed in #116