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(router): do not run CD cycles unnecessary when markdown modules are loaded #557

Merged
merged 1 commit into from
Jul 23, 2023
Merged

Conversation

arturovt
Copy link
Contributor

Hey, I've just started experimenting with Analog and counting change detection cycles occurring in different scenarios due to the various asynchronous operations used in different parts of the code. I noticed that rendering a simple page, like about.md from the tutorial, causes 8 ticks immediately. This current commit updates the implementation of the toMarkdownModule, which loads @analogjs/content and markdownFileFactory, scheduling tasks in the root zone.

@netlify
Copy link

netlify bot commented Jul 23, 2023

Deploy Preview for analog-app ready!

Name Link
🔨 Latest commit 0bd8a1d
🔍 Latest deploy log https://app.netlify.com/sites/analog-app/deploys/64bd6533c3bee400089cde61
😎 Deploy Preview https://deploy-preview-557--analog-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jul 23, 2023

Deploy Preview for analog-docs ready!

Name Link
🔨 Latest commit 0bd8a1d
🔍 Latest deploy log https://app.netlify.com/sites/analog-docs/deploys/64bd6533f74eca0008f7c843
😎 Deploy Preview https://deploy-preview-557--analog-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jul 23, 2023

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 0bd8a1d
🔍 Latest deploy log https://app.netlify.com/sites/analog-blog/deploys/64bd6533f112870008ea663f
😎 Deploy Preview https://deploy-preview-557--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@brandonroberts
Copy link
Member

Thanks @arturovt. Will you share how you track the CD cycles?

@arturovt
Copy link
Contributor Author

Sure. Firstly, I start with patching tick(), which is the simplest way to track the number of change detections:

const tick = ApplicationRef.prototype.tick;
let ticksCalled = 0;
ApplicationRef.prototype.tick = function () {
  console.log(`Ticks called: ${ticksCalled++}`);
  return tick.call(this);
};

But this would only tell me the number of CDs happening.

Then I track asynchronous tasks which are invoked in the Angular zone. I usually just add a console logging to node_modules/@angular/core/fesm2022/core.mjs -> forkInnerZoneWithAngularBehavior -> onInvokeTask: (delegate, current, target, task, applyThis, applyArgs), I add the call before onEnter(zone):

console.log(task);
onEnter(zone);

It'is easy to track event tasks since they have a source property (for example, HTMLAnchorElement:click).

It's always challenging to track microtasks because zone.js intercepts all calls to new Promise, Promise.prototype.then, and Promise.resolve. then and resolve typically lead to the creation of new Promise instances. zone.js replaces globalThis.Promise with its own implementation called ZoneAwarePromise, which affects debugging capabilities. In zone.js, there is a function called scheduleResolveOrReject where we may find the following code:

zone.scheduleMicroTask(source, () => {
    try {
        const parentPromiseValue = promise[symbolValue];

The promise may be the original promise that resulted in a microtasks chain. However, this is just a surface-level observation, as it often takes some time to identify the root cause of the change detection triggering.

See also this code as an example how tasks can be easily tracked: https://ng-guru.io/guide/change-detection-cycles-per-different-async-apis/#tracing-asynchronous-tasks

@brandonroberts brandonroberts merged commit 7646549 into analogjs:main Jul 23, 2023
22 checks passed
@arturovt arturovt deleted the fix/router-cds branch July 23, 2023 19:51
Villanuevand pushed a commit to Villanuevand/analog that referenced this pull request Sep 12, 2023
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.

None yet

2 participants