Skip to content
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

Update core dependencies #1891

Closed

Conversation

typescript-bot
Copy link
Contributor

Automated changes by create-pull-request GitHub action

Copy link
Contributor

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

@saschanaz
Copy link
Contributor

mdn/browser-compat-data#25733 🤔

@@ -18266,8 +18266,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

MDN hasn't been updated yet, but I don't think the BCD PR is right.

Choose a reason for hiding this comment

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

I don't understand why we're deleting this? responseStart has been supported for a long time. It's just what "responseStart" mean changed slightly in Chrome 115, and has been changed back since Chrome 133. See: https://chromestatus.com/feature/5158830722514944. That's why we updated BCD to make a noted of that.

But in terms of typescript, reponseStart was always present and was always a DOMHighResTimeStamp. So that note shouldn't change anything in typescript.

@typescript-bot typescript-bot force-pushed the update-core-deps branch 4 times, most recently from d0f76ee to cc1ade5 Compare February 6, 2025 08:10
@@ -4336,8 +4345,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;

Choose a reason for hiding this comment

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

Add back incorrectly removed responseStart

Suggested change
readonly responseEnd: DOMHighResTimeStamp;
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;

@@ -4208,8 +4217,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;

Choose a reason for hiding this comment

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

Add back incorrectly removed responseStart

Suggested change
readonly responseEnd: DOMHighResTimeStamp;
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;

@@ -4336,8 +4345,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;

Choose a reason for hiding this comment

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

Add back incorrectly removed responseStart

Suggested change
readonly responseEnd: DOMHighResTimeStamp;
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;

@@ -4208,8 +4217,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;

Choose a reason for hiding this comment

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

Add back incorrectly removed responseStart

Suggested change
readonly responseEnd: DOMHighResTimeStamp;
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;

@@ -4910,8 +4921,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;

Choose a reason for hiding this comment

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

Add back incorrectly removed responseStart

Suggested change
readonly responseEnd: DOMHighResTimeStamp;
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;

@@ -4910,8 +4921,6 @@ interface PerformanceResourceTiming extends PerformanceEntry {
readonly requestStart: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseEnd) */
readonly responseEnd: DOMHighResTimeStamp;

Choose a reason for hiding this comment

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

Add back incorrectly removed responseStart:

Suggested change
readonly responseEnd: DOMHighResTimeStamp;
readonly responseEnd: DOMHighResTimeStamp;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/PerformanceResourceTiming/responseStart) */
readonly responseStart: DOMHighResTimeStamp;

@saschanaz
Copy link
Contributor

This is because now surprisingly there are two sources of truth in BCD for responseStart. I strongly believe BCD should remove that, but until then we have a way to override BCD data.

@tunetheweb
Copy link

This is because now surprisingly there are two sources of truth in BCD for responseStart. I strongly believe BCD should remove that, but until then we have a way to override BCD data.

Can you explain what you mean by that? I did the BCD updates so can see if there's a better way to do this (I'm not totally happy how it shows on MDN either).

But, as per, #1891 (comment) the point was to note that Chrome diverged from the definition for a short period and this might surprise some developers—hence why we wanted it documented in MDN/BCD.

@tunetheweb
Copy link

Reverting the problematic updates in BCD here: #25876

@saschanaz
Copy link
Contributor

This is because now surprisingly there are two sources of truth in BCD for responseStart. I strongly believe BCD should remove that, but until then we have a way to override BCD data.

Can you explain what you mean by that? I did the BCD updates so can see if there's a better way to do this (I'm not totally happy how it shows on MDN either).

Here the two sources means the support data with alternative_name from the finalResponseHeadersStart and the data from the responseStart.

  • We have to consider both data because alternative_name is often about things that are still supported by browsers.
  • alternative_name had always been the sole source for the name before the responseStart change.
  • After the change, we have data from two sources saying removal and addition, and the code is not ready for that

We can somehow deal with it by merging support data (we have that for mixins), but I don't think that's worth given responseStart is the only source of conflict we had for years.

@tunetheweb
Copy link

Looks like the responseStart deletions have been removed with that latest update from BCD.

@saschanaz saschanaz mentioned this pull request Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants