+
Skip to content

Conversation

krystofwoldrich
Copy link
Contributor

@krystofwoldrich krystofwoldrich commented Aug 30, 2024

Based on the spec and the current implementation the second argument of DOMParser.parseFromString is not optional and doesn't have a default value.

throw new TypeError('DOMParser.parseFromString: the provided mimeType "' + mimeType + '" is not valid.');

https://developer.mozilla.org/en-US/docs/Web/API/DOMParser/parseFromString
https://html.spec.whatwg.org/#dom-domparser-parsefromstring-dev

This fixes types for changes made in https://github.com/xmldom/xmldom/releases/tag/0.9.0

Previous release https://github.com/xmldom/xmldom/releases/tag/0.8.10 allowed mimeType as optional.

Copy link
Member

@karfau karfau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG, thx

Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.26%. Comparing base (4876807) to head (6d4abbe).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #713   +/-   ##
=======================================
  Coverage   94.26%   94.26%           
=======================================
  Files           8        8           
  Lines        2094     2094           
  Branches      537      537           
=======================================
  Hits         1974     1974           
  Misses        120      120           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@karfau karfau merged commit b1f133c into xmldom:master Aug 30, 2024
@karfau
Copy link
Member

karfau commented Aug 30, 2024

I will file a release within the next few days.

@karfau karfau added bug Something isn't working types Anything regarding Typescript labels Aug 30, 2024
@karfau karfau added this to the 0.9.1 milestone Aug 30, 2024
@karfau
Copy link
Member

karfau commented Sep 2, 2024

@krystofwoldrich FYI: it will still take some time, since I want to avoid a ton of releases with just type fixes, so I'm working on #717 right now, which will fix all currently known type issues.

As part of that I also widened the accepted type to a literal type | string, just in case somebody only has that as a type for the mimeType to pass.

@papandreou
Copy link

Would be really nice to get a new release out soon, as the latest version throws an error when you don't supply the mimeType arg, which makes it incompatible with things like saml2-js? That seems more important to fix than typings?

@karfau
Copy link
Member

karfau commented Sep 3, 2024

@papandreou this is intentional breaking change in version 0.9.0 which aligns the behavior of xmldom with the specs. This is not going to be "fixed", since it works as expected.
If the previous behavior is required you can not upgrade and have to stick to 0.8.*!
I do not recommend sticking to this behavior, since it means the content that you are parsing is used to determine the mime type. Which is only secure if you know what kind of content you expect, in which case you can also put the mime type as an argument.

The issue described here, is that this change is not reflected in the types of 0.9.0, which will be fixed along with all other type issues as part of the PR linked in my previous comment.

for further discussion on this topic I would prefer to have it in the release discussion #435

@papandreou
Copy link

Ah, apologies, I thought it was the other way around 🙈

@mogadanez
Copy link

hmmm... side note - should such changes be in major releases?
Today catch production issues because of this change after "silent" upgrade of this package.

@karfau
Copy link
Member

karfau commented Feb 4, 2025

In 0.* (aka unstable) versions, breaking changes are happening in minor versions.
There are tools like Renovate, that are aware is aware of this.
Sorry that you were not aware of it, and were affected by this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working types Anything regarding Typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载