[APM] Make typescript optimization process compatible with NP #58984
Conversation
dgieselaar
force-pushed
the
apm-tsconfig-np
branch
from
3236e24 to
9a02a92
Compare
|
|
||
| await writeFile( | ||
| xpackTsConfig, | ||
| JSON.stringify(assign(config, template), null, 2), |
There was a problem hiding this comment.
whoa, haven't seen _.assign used since the ancient es5 times :D I must admit, I thought it was only a legacy Object.assign.
Would this not work?
| JSON.stringify(assign(config, template), null, 2), | |
| JSON.stringify({...config, ...template}, null, 2), |
There was a problem hiding this comment.
aye, I was changing it from merge, so lodash was still living in my head 😬 will change.
Rather than creating an extra tsconfig.json file in the APM folder, simply change the one in x-pack root, and include APM files from both legacy + NP.
dgieselaar
force-pushed
the
apm-tsconfig-np
branch
from
9a02a92 to
b0b181d
Compare
dgieselaar
added
release_note:skip
| optimizeTsConfig(); | ||
| optimizeTsConfig().catch(err => { | ||
| // eslint-disable-next-line no-console | ||
| console.log(err); |
There was a problem hiding this comment.
If you do console.error here will ESLint not complain? You would want this on stderr anyway I presume.
There was a problem hiding this comment.
good point, I'll move it to console.error (regardless of whether it will complain)
| unoptimizeTsConfig: async () => { | ||
| await unoptimizeTsConfig(); | ||
| // eslint-disable-next-line no-console | ||
| console.log('Removed APM TypeScript optimizations'); |
There was a problem hiding this comment.
This should go to stderr too.
There was a problem hiding this comment.
It's not an error though, am I misunderstanding what you mean?
There was a problem hiding this comment.
console.error logs to stderr instead of stdout. stdout is meant for output that can be piped around, and stderr for messages like this.
https://www.jstorimer.com/blogs/workingwithcode/7766119-when-to-use-stderr-instead-of-stdout
it's probably a controversial point though against "console.error is for errors" so either way I guess.
There was a problem hiding this comment.
I got that, but you might have commented in the wrong file? Did you meant to comment on scripts/unoptimize-tsconfig.js? (I've changed it to console.error there as well).
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…c#58984) * [APM] Make typescript optimization process compatible with NP Rather than creating an extra tsconfig.json file in the APM folder, simply change the one in x-pack root, and include APM files from both legacy + NP. * Update dev_docs/typescript.md * Use spread op instead of assign * Use console.error instead of console.log
Rather than creating an extra tsconfig.json file in the APM folder, simply change the one in x-pack root, and include APM files from both legacy + NP.