Skip to content

Commit

Permalink
fix(cli-hooks): silence node warnings that can break @slack/cli-hooks (
Browse files Browse the repository at this point in the history
  • Loading branch information
mwbrooks authored Nov 21, 2024
1 parent 278514f commit c5f8ffd
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 8 deletions.
2 changes: 1 addition & 1 deletion packages/cli-hooks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Below is an example `slack.json` file that overrides the default `start` hook:
```json
{
"hooks": {
"get-hooks": "npx -q --no-install -p @slack/cli-hooks slack-cli-get-hooks",
"get-hooks": "NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-get-hooks",
"start": "npm run dev"
}
}
Expand Down
8 changes: 5 additions & 3 deletions packages/cli-hooks/src/get-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ if (fs.realpathSync(process.argv[1]) === fileURLToPath(import.meta.url)) {
* @returns {SDKInterface} Information about the hooks currently supported.
*/
export default function getHooks() {
// NODE_NO_WARNINGS=1 silences node process warnings. These warnings can occur
// on some node version (e.g. v23.2.0) and cause invalid JSON hook responses.
return {
hooks: {
doctor: 'npx -q --no-install -p @slack/cli-hooks slack-cli-doctor',
'check-update': 'npx -q --no-install -p @slack/cli-hooks slack-cli-check-update',
'get-manifest': 'npx -q --no-install -p @slack/cli-hooks slack-cli-get-manifest',
doctor: 'NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-doctor',
'check-update': 'NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-check-update',
'get-manifest': 'NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-get-manifest',
start: 'npx -q --no-install -p @slack/cli-hooks slack-cli-start',
},
config: {
Expand Down
10 changes: 7 additions & 3 deletions packages/cli-hooks/src/get-hooks.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ import getHooks from './get-hooks.js';
describe('get-hooks implementation', async () => {
it('should return scripts for required hooks', async () => {
const { hooks } = getHooks();
assert(hooks.doctor === 'npx -q --no-install -p @slack/cli-hooks slack-cli-doctor');
assert(hooks['check-update'] === 'npx -q --no-install -p @slack/cli-hooks slack-cli-check-update');
assert(hooks['get-manifest'] === 'npx -q --no-install -p @slack/cli-hooks slack-cli-get-manifest');
assert(hooks.doctor === 'NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-doctor');
assert(
hooks['check-update'] === 'NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-check-update',
);
assert(
hooks['get-manifest'] === 'NODE_NO_WARNINGS=1 npx -q --no-install -p @slack/cli-hooks slack-cli-get-manifest',
);
assert(hooks.start === 'npx -q --no-install -p @slack/cli-hooks slack-cli-start');
});

Expand Down
2 changes: 1 addition & 1 deletion packages/cli-hooks/src/protocols.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function DefaultProtocol(args) {
// If the particular hook invocation is requesting manifest generation we
// ensure any logging is a no-op to prevent littering stdout with logging
// and confusing the CLI's manifest JSON payload parsing.
const loggerMethod = manifestOnly ? () => { } : console.log;
const loggerMethod = manifestOnly ? () => {} : console.log;
return {
name: DEFAULT_PROTOCOL,
log: loggerMethod,
Expand Down

0 comments on commit c5f8ffd

Please sign in to comment.