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.

Sign up for GitHub

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

Jump to bottom

perf_hooks: add deliveryType and responseStatus fields #51589

Merged
merged 3 commits into from
May 12, 2024
Merged

perf_hooks: add deliveryType and responseStatus fields #51589

merged 3 commits into from
May 12, 2024

Conversation

KhafraDev
Copy link
Member

Fixes #51001

undici will already work once this change lands.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 29, 2024
KhafraDev
KhafraDev commented Jan 29, 2024
View reviewed changes
lib/internal/perf/resource_timing.js
initiatorType,
timingInfo,
cacheMode = '',
bodyInfo,
Copy link
Member Author

Choose a reason for hiding this comment

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

bodyInfo is currently unused because there's a possible bug where the resource timing is taking fields like encodedBodySize/decodedBodySize from the resource's timing info rather than its resource info.

MmdDebian
MmdDebian reviewed Jan 29, 2024
View reviewed changes
Copy link

@MmdDebian MmdDebian left a comment

Choose a reason for hiding this comment

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

That is cool

H4ad
H4ad approved these changes Jan 29, 2024
View reviewed changes
Copy link
Member

@H4ad H4ad left a comment

Choose a reason for hiding this comment

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

Can you update the documentation too?

node/doc/api/perf_hooks.md

Line 249 in 94f824a

### `performance.markResourceTiming(timingInfo, requestedUrl, initiatorType, global, cacheMode)`

Also, adding those fields is not a breaking change?

@KhafraDev
Copy link
Member Author

No, they're expected to exist but they weren't implemented.

@H4ad H4ad added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56956/

anonrig
anonrig approved these changes Jan 29, 2024
View reviewed changes
doc/api/perf_hooks.md Show resolved Hide resolved
@KhafraDev KhafraDev mentioned this pull request Jan 29, 2024
Merged
7 tasks
mcollina
mcollina approved these changes Jan 30, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@H4ad H4ad added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57117/

@legendecas legendecas added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Mar 5, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57586/

@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58362/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59082/

@KhafraDev KhafraDev added the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59107/

@KhafraDev KhafraDev added the request-ci Add this label to start a Jenkins CI on a PR. label May 12, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 12, 2024
@nodejs-github-bot

This comment was marked as duplicate.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59157/

@aduh95 aduh95 merged commit e2697c1 into nodejs:main May 12, 2024
64 of 65 checks passed
@aduh95
Copy link
Contributor

aduh95 commented May 12, 2024

Landed in e2697c1

@KhafraDev KhafraDev deleted the performance-markresource-timing branch May 12, 2024 19:13
targos pushed a commit that referenced this pull request May 12, 2024
@KhafraDev @targos
perf_hooks: add deliveryType and responseStatus fields
1f7c2a9 
PR-URL: #51589
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@targos
Copy link
Member

targos commented May 13, 2024

This PR adds a feature so it should be labeled semver-minor PRs that contain new features and should be released in the next minor version. (this is not supposed to be done by releasers).

targos added a commit that referenced this pull request May 13, 2024
@targos
2024-05-14, Version 22.2.0 (Current)
7d92816 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    #52766
doc:
  * add pimterry to collaborators (Tim Perry)
    #52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) #52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    #52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    #52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) #51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    #52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    #52692

PR-URL: #52971
@targos targos mentioned this pull request May 13, 2024
Merged
@targos targos added the semver-minor PRs that contain new features and should be released in the next minor version. label May 15, 2024
targos added a commit that referenced this pull request May 15, 2024
@targos
2024-05-15, Version 22.2.0 (Current)
d57af10 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    #52766
doc:
  * add pimterry to collaborators (Tim Perry)
    #52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) #52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    #52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    #52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) #51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    #52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    #52692

PR-URL: #52971
targos added a commit that referenced this pull request May 15, 2024
@targos
2024-05-15, Version 22.2.0 (Current)
28b0a58 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    #52766
doc:
  * add pimterry to collaborators (Tim Perry)
    #52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) #52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    #52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    #52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) #51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    #52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    #52692

PR-URL: #52971
sophonieb pushed a commit to sophonieb/node that referenced this pull request Jun 20, 2024
@KhafraDev @sophonieb
perf_hooks: add deliveryType and responseStatus fields
0f9917a 
PR-URL: nodejs#51589
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
sophonieb pushed a commit to sophonieb/node that referenced this pull request Jun 20, 2024
@targos @sophonieb
2024-05-15, Version 22.2.0 (Current)
eccd98f 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    nodejs#52766
doc:
  * add pimterry to collaborators (Tim Perry)
    nodejs#52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) nodejs#52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    nodejs#52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    nodejs#52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) nodejs#51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    nodejs#52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    nodejs#52692

PR-URL: nodejs#52971
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@KhafraDev @bmeck
perf_hooks: add deliveryType and responseStatus fields
0d01896 
PR-URL: nodejs#51589
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@targos @bmeck
2024-05-15, Version 22.2.0 (Current)
99e317e 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    nodejs#52766
doc:
  * add pimterry to collaborators (Tim Perry)
    nodejs#52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) nodejs#52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    nodejs#52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    nodejs#52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) nodejs#51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    nodejs#52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    nodejs#52692

PR-URL: nodejs#52971
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024
@skyclouds2001 skyclouds2001 mentioned this pull request Nov 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MmdDebian MmdDebian MmdDebian left review comments

@H4ad H4ad H4ad approved these changes

@legendecas legendecas legendecas approved these changes

@JungMinu JungMinu JungMinu approved these changes

@anonrig anonrig anonrig approved these changes

@lpinca lpinca lpinca approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-ci PRs that need a full CI run. perf_hooks Issues and PRs related to the implementation of the Performance Timing API. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

performance.markResourceTiming is outdated