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

Fix DEP0152 / deprecated PerformanceEntry.kind usage #440

Merged
merged 1 commit into from
Jul 31, 2021

Conversation

sirreal
Copy link
Contributor

@sirreal sirreal commented May 12, 2021

Fix #435

Node 16 deprecated PerformanceEntry.kind, moving it to PerformanceEntry.detail.kind.

Use entry.detail.kind if available. Fall back to deprecated entry.kind.

See https://nodejs.org/docs/latest-v16.x/api/deprecations.html#deprecations_dep0152_extension_performanceentry_properties

@analytik
Copy link

While I'm grateful for this, maybe we should also fix the deprecated format and an ignored parameter, see the node.js bug comment.

If think if (entry.detail.kind) passes, we should assume a newer Node.js version and also instead of { entryTypes: ['gc'], buffered: false } use { type: 'gc' } - it will likely be more future-proof.

@sirreal
Copy link
Contributor Author

sirreal commented May 13, 2021

Sure, buffered: false appears to be completely redundant as it was the default value and has now been removed. I'll push a change for that.

Regarding type/entryTypes, I understand these are synonymous, where the second is compatible with older versions:

obs.observe({ type: 'gc' });
obs.observe({ entryTypes: ['gc'] });

{ type: 'gc' } may be the preferred version for observing a single type, but { entryTypes: ['gc'] } remains a valid way of observing one or more types according to the v16 documentation.

I don't believe checking event.detail for this will work. We would need to know whether to use type or entryTypes before we start observing, and we only see event after we've started observing. We may have to resort to inspecting process.version. I'd leave this as-is.

It may be worth asking for clarification of nodejs/node#38412 (comment) around type/entryTypes. Would you like me to do that?

@sirreal
Copy link
Contributor Author

sirreal commented May 13, 2021

I dug a bit into the specs because nodejs/node#38412 (comment) mentioned aligning with them.

Both MDN and the w3c spec describe type and entryTypes. The difference seems to set the observer type to "single" or "multiple", respectively. It's unclear to me what the implications are or if there are downsides to a "multiple" type observer observing a single type.

The spec also mentions buffered, so perhaps I'm not looking in the right place.

Node 16 moved the PerformancyEntry.
See https://nodejs.org/docs/latest-v16.x/api/deprecations.html#deprecations_dep0152_extension_performanceentry_properties

Remove `buffered` observe option

This option has been removed in Node 16:
https://nodejs.org/docs/latest-v16.x/api/perf_hooks.html#perf_hooks_performanceobserver_observe_options

This was redundant in Node <16 because `false` was already the default:
https://nodejs.org/docs/latest-v15.x/api/perf_hooks.html#perf_hooks_performanceobserver_observe_options

> buffered <boolean> … Default: false.

Remove buffering comment
@zbjornson zbjornson force-pushed the fix/435-gc-kind-deprecation branch 2 times, most recently from 8810e74 to ec4b5e0 Compare July 31, 2021 17:44
Copy link
Collaborator

@zbjornson zbjornson left a comment

Choose a reason for hiding this comment

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

Excellent, thank you. Sorry for the slow review.

I pushed to update the changelog and get CI to run.

@zbjornson zbjornson merged commit c31ccec into siimon:master Jul 31, 2021
@gruselhaus
Copy link

I'm running node v16.6.1 along with prom-client v13.1.0 and still get this deprecation warning.

Bildschirmfoto 2021-08-08 um 16 45 02

Is anyone able to reproduce this?

@zbjornson
Copy link
Collaborator

@gruselhaus sorry, I just published v13.2.0 with the fix.

@sirreal sirreal deleted the fix/435-gc-kind-deprecation branch October 20, 2023 14:10
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.

DeprecationWarning when running Node 16
4 participants