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

doc: add esm examples to node:perf_hooks #55257

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

mfdebian
Copy link
Contributor

@mfdebian mfdebian commented Oct 3, 2024

This PR adds the ESM counterparts of the CJS examples for the Performance measurement APIs documentation.

There is one particular example in which I'd like to get a more in depth feedback, it's the Measuring how long it takes to load dependencies example.

I've done it this way:

import { performance, PerformanceObserver } from 'node:perf_hooks';

// Activate the observer
const obs = new PerformanceObserver((list) => {
  const entries = list.getEntries();
  entries.forEach((entry) => {
    console.log(`import('${entry[0]}')`, entry.duration);
  });
  performance.clearMarks();
  performance.clearMeasures();
  obs.disconnect();
});
obs.observe({ entryTypes: ['function'], buffered: true });

const timedImport = performance.timerify(async (module) => {
  return await import(module);
});

await timedImport('some-module');

(you can try it by replacing 'some-module' with, let's say, 'node:child_process')

Which gets the job done, but would you agree it's a correct counterpart for its CJS example? I feel I might lack more deep knowledge about the differences between require and import.

Thanks in advance and as always best regards!

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. perf_hooks Issues and PRs related to the implementation of the Performance Timing API. labels Oct 3, 2024
@lpinca
Copy link
Member

lpinca commented Oct 11, 2024

@nodejs/loaders for the question.

@mfdebian
Copy link
Contributor Author

mfdebian commented Dec 5, 2024

hello @lpinca do you know of anyone I could maybe tag to get a review on this PR? just so it doesn't get stalled 😊

thanks in advance! 🙌

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

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 12, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 12, 2024
@nodejs-github-bot nodejs-github-bot merged commit 96cd2a6 into nodejs:main Dec 12, 2024
26 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 96cd2a6

targos pushed a commit that referenced this pull request Dec 13, 2024
aduh95 pushed a commit that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants