-
Notifications
You must be signed in to change notification settings - Fork 798
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(mock-doc): Add missing DOMParser stub to MockWindow #3279
Conversation
Ok, I'm pretty confused here. I can't build stencil, i thought it was an environment problem so i spun up an ubuntu 20.04 docker image and installed nodejs 16.14.0 just like the CI, but it still won't build. I get through installing the deps with Output# npm run build -- --ci |
@cam-narzt can you run |
Thanks, that helped. |
Prettier keeps failing with:
But I don't know what that means. |
It looks like |
Co-authored-by: Ryan Waskiewicz <ryanwaskiewicz@gmail.com>
Co-authored-by: Ryan Waskiewicz <ryanwaskiewicz@gmail.com>
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.
Looks good to me, thanks!
src/mock-doc/parser.ts
Outdated
| 'image/svg+xml'; | ||
|
||
export class MockDOMParser { | ||
parseFromString(string: string, type: DOMParserSupportedType): MockDocument { |
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.
Since 'string' and 'type' are both technically reserved words in TypeScript, can we change these to use names that don't overlap with reserved keywords and are a little more descriptive?
parseFromString(string: string, type: DOMParserSupportedType): MockDocument { | |
parseFromString(htmlToParse: string, mimeType: DOMParserSupportedType): MockDocument { |
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.
(GitHub seems to be having issues with duplicate comments this afternoon, this wasn't around when I hit the approve button - sorry about that - no action needed here)
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build
) was run locally and any changes were pushednpm test
) were run locally and passednpm run test.karma.prod
) were run locally and passednpm run prettier
) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Testing code which attempts to use the DOMParser API will fail, because it isn't made available in the test environment.
GitHub Issue Number: N/A
What is the new behavior?
The DOMParser now has a minimal stub which can handle parsing HTML, but not yet XML.
Does this introduce a breaking change?
Testing
Other information