fix(v2): Fix update-notifier not run at first and not notifying consistently #5110
Conversation
|
✔️ [V2] 🔨 Explore the source changes: cbc6ceb 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60ddc1871a17c5000794fdf2 😎 Browse the preview: https://deploy-preview-5110--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5110--docusaurus-2.netlify.app/ |
|
Size Change: 0 B Total Size: 635 kB ℹ️ View Unchanged
|
| // Note: the notification will only happen in the 2nd run | ||
| // See https://github.com/yeoman/update-notifier/issues/209 | ||
| if ( | ||
| !notifier.disabled && |
There was a problem hiding this comment.
@slorber I didn't find this field anywhere in the docs...
There was a problem hiding this comment.
this update notifier package is a bit complicated, you'd rather read source code to understand how it works 😅
There was a problem hiding this comment.
The reason is because I'm touching that source code a little and ts-check throws an error because this field is not in the typedef. That means it's not public API? I do see that exists, just not sure if it's actually useful...
There was a problem hiding this comment.
pretty sure we had failures without that check, I'd rather not change something that works just to please TS ;)
There was a problem hiding this comment.
Yeah, I clearly see that. I'm not saying to remove it, just curious how you dug it up if it's neither typedefed nor documented.
Motivation
Motived by this comment: #5105 (comment)
Have you read the Contributing Guidelines on pull requests?
yes
Test Plan
local tests