-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: pass environmentOptions to happy-dom integration #3902
Conversation
Run & review this pull request in StackBlitz Codeflow. |
✅ Deploy Preview for fastidious-cascaron-4ded94 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I have closed #3907 now as this PR has added the default URL to it. |
We should add a test |
@userquin I've added them. Let me know if it's enough |
it('accepts custom environment options', () => { | ||
// default is false | ||
expect((window.happyDOM as any).settings.disableCSSFileLoading).toBe(true) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we leverage on passing envrionment options via JSDoc code block, I didn't find a way where I could test passing the option vs not passing.
} | ||
|
||
it('defaults URL to localhost:3000', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same applies here.
I'd like to try passing and not passing URL, but no idea how to do that using JSDoc comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can create a separate test file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, can you add types to environmentOptions
in the config?
@@ -5,9 +5,12 @@ import { populateGlobal } from './utils' | |||
export default <Environment>({ | |||
name: 'happy-dom', | |||
transformMode: 'web', | |||
async setupVM() { | |||
async setupVM({ 'happy-dom': happyDom = {} }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think naming it happyDOM
in the config is fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The types can be found here:
import IHappyDOMOptions from 'happy-dom/lib/window/IHappyDOMOptions.js';
The source code for them is here:
https://github.com/capricorn86/happy-dom/blob/master/packages/happy-dom/src/window/IHappyDOMOptions.ts
} | ||
|
||
it('defaults URL to localhost:3000', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can create a separate test file
I'm going to do a release in a few minutes, but you didn't allow maintainers to push to your PR, so I created a separate one: #3972 Thank you for your contribution! |
Description
This PR forwards the
environmentOptions
tohappy-dom
integration.It follows the same principle defined in the
jsdom
integrationvitest/packages/vitest/src/integrations/env/jsdom.ts
Line 32 in f4e6e99
Closes #3903
Closes #3905
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.