capricorn86 / happy-dom

A JavaScript implementation of a web browser without its graphical user interface
MIT License
3.25k stars 197 forks source link

feat(jest-environment): Support configuration of happy-dom with testEnvironmentOptions #1287

Closed Codex- closed 6 months ago

Codex- commented 6 months ago

This PR allows users of @happy-dom/jest-environment to pass through non-deprecated configuration options from IBrowserSettings to happy-dom via testEnvironmentOptions in the jest config.

This retains the existing behaviour of setting the default localhost URL on the instance.

Looking into testing, since the existing tests for this package are configured more like integration tests what would your preferred testing situation be? I add unit tests and set that up for this package to test those? Or add a new integration suite?

Codex- commented 6 months ago

Instead of setting each option on the "window.happyDOM" property, I changed so that the object is sent into the Window constructor directly instead.

I did consider this but went the route of offering feedback to the consumer should the configuration be invalid. If you typo a key or use an invalid option, does this get bubbled up by happydom still?

I hope you are fine with the solution 🙂

It's simpler but at the cost of providing feedback to the consumer, and forces errorCapture to disabled.

The usage of errorCapture is actually my main motivator for adding this, as I have been trying to test error boundaries with react-router-dom so setting this to disabled works for my case.

I feel as though it should respect the setting specified in the config instead of being force disabled https://github.com/capricorn86/happy-dom/pull/1287/files#diff-ca9251b53f292fc136910984f98fa20367077cf2f260010ca5cf707d047a6a69R57

Flipping it to:

            settings: {
                errorCapture: BrowserErrorCaptureEnum.disabled,
                ...(<IOptionalBrowserSettings>projectConfig.testEnvironmentOptions?.settings)
            }

Would respect the user-defined option for this

capricorn86 commented 6 months ago

Instead of setting each option on the "window.happyDOM" property, I changed so that the object is sent into the Window constructor directly instead.

I did consider this but went the route of offering feedback to the consumer should the configuration be invalid. If you typo a key or use an invalid option, does this get bubbled up by happydom still?

I hope you are fine with the solution 🙂

It's simpler but at the cost of providing feedback to the consumer, and forces errorCapture to disabled.

The usage of errorCapture is actually my main motivator for adding this, as I have been trying to test error boundaries with react-router-dom so setting this to disabled works for my case.

I feel as though it should respect the setting specified in the config instead of being force disabled https://github.com/capricorn86/happy-dom/pull/1287/files#diff-ca9251b53f292fc136910984f98fa20367077cf2f260010ca5cf707d047a6a69R57

Flipping it to:

          settings: {
              errorCapture: BrowserErrorCaptureEnum.disabled,
              ...(<IOptionalBrowserSettings>projectConfig.testEnvironmentOptions?.settings)
          }

Would respect the user-defined option for this

I think that it is probably better to add the validation of the settings in Window in that case, but it will affect performance. If we would add the validation in this package, it would require automated tests to ensure that it will not break as it is complex logic. Adding unit tests for this package would probably require some refactoring to separate integration tests from unit tests.

I put errorCapture after to ensure that it will not be set. The reason is that process level error capturing will not work at all (as it collides with Jest error capturing), and try/catch will capture some errors in tests, but Jest will not be able to detect them, leading to weird behaviors.