Ensure generation and related utils are sync #9692
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9692 +/- ##
==========================================
+ Coverage 86.11% 86.13% +0.01%
==========================================
Files 327 327
Lines 12512 12548 +36
==========================================
+ Hits 10775 10808 +33
- Misses 1737 1740 +3
Continue to review full report at Codecov.
|
pgrzesik
force-pushed
the
refactor-telemetry-generation-to-sync
branch
from
1735803
to
9dec03c
Compare
pgrzesik
changed the title
lib/utils/npmPackage/isGlobal.js
Outdated
| module.exports = memoizee(() => { | ||
| const npmPackagesRoot = (() => { | ||
| try { | ||
| return String(spawnSync('npm', ['root', '-g']).stdoutBuffer).trim(); |
There was a problem hiding this comment.
In spawnSync it's stdout that's stdout buffer not stdoutBuffer (in child-process-ext/spawn, stdout is a stream, while stdoutBuffer is a buffer)
https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options
There was a problem hiding this comment.
Great catch, thank you
scripts/postinstall.js
Outdated
| @@ -22,7 +22,7 @@ const isNpmGlobalPackage = require('../lib/utils/npmPackage/isGlobal'); | |||
| messageTokens.push('To start your first project run “serverless”.'); | |||
| } | |||
|
|
|||
| if ((isStandaloneExecutable && !isWindows) || (await isNpmGlobalPackage())) { | |||
| if ((isStandaloneExecutable && !isWindows) || isNpmGlobalPackage()) { | |||
There was a problem hiding this comment.
If I see correctly, we can drop async func wrapper now
There was a problem hiding this comment.
This wrapper was introduced only for sake of using await, so now can be dropped entirely
scripts/serverless.js
Outdated
| @@ -697,14 +697,14 @@ const processSpanPromise = (async () => { | |||
| hasTelemetryBeenReported = true; | |||
| if (!isTelemetryDisabled && !isHelpRequest && serverless.isTelemetryReportedExternally) { | |||
| await storeTelemetryLocally({ | |||
There was a problem hiding this comment.
This await can also be dropped
pgrzesik
force-pushed
the
refactor-telemetry-generation-to-sync
branch
from
9dec03c
to
a021ba7
Compare
scripts/postinstall.js
Outdated
| @@ -22,7 +22,7 @@ const isNpmGlobalPackage = require('../lib/utils/npmPackage/isGlobal'); | |||
| messageTokens.push('To start your first project run “serverless”.'); | |||
| } | |||
|
|
|||
| if ((isStandaloneExecutable && !isWindows) || (await isNpmGlobalPackage())) { | |||
| if ((isStandaloneExecutable && !isWindows) || isNpmGlobalPackage()) { | |||
There was a problem hiding this comment.
This wrapper was introduced only for sake of using await, so now can be dropped entirely
pgrzesik
force-pushed
the
refactor-telemetry-generation-to-sync
branch
from
a021ba7
to
b19a7ff
Compare
Discussed internally, the reason behind this change is to ensure that both
storeLocallyandgeneratePayloadutils are sync, which makes them safe to be executed as a part ofprocess.onhandler without interfering with asynchronously running operations in the backgorund.Related to: #9367