Build: less noise when compiling #19808
Conversation
| if (!optimized) { | ||
| await Promise.all( | ||
| entries.map(async (file: string) => { | ||
| console.log(`skipping generating types for ${file}`); | ||
| const { name: entryName, dir } = path.parse(file); | ||
|
|
||
| const pathName = join(process.cwd(), dir.replace('./src', 'dist'), `${entryName}.d.ts`); | ||
| const srcName = join(process.cwd(), file); | ||
|
|
||
| const rel = relative(dirname(pathName), dirname(srcName)) | ||
| .split(path.sep) | ||
| .join(path.posix.sep); | ||
|
|
||
| await fs.ensureFile(pathName); | ||
| await fs.writeFile( | ||
| pathName, | ||
| dedent` | ||
| // devmode | ||
| export * from '${rel}/${entryName}'; | ||
| ` | ||
| ); | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Moving this should get a little speed increase, since we're not waiting for this to complete before starting the esbuild builds.
| build({ | ||
| silent: true, |
There was a problem hiding this comment.
Making it silent, will only output errors
| c.define = optimized | ||
| ? { | ||
| 'process.env.NODE_ENV': "'production'", | ||
| 'process.env': '{}', | ||
| global: 'window', | ||
| } | ||
| : { | ||
| 'process.env.NODE_ENV': "'development'", | ||
| 'process.env': '{}', | ||
| global: 'window', | ||
| }; |
There was a problem hiding this comment.
I have a suspicion that this is not required anymore
| entry: entries.map((e: string) => slash(join(cwd, e))), | ||
| watch, | ||
| ...(tsConfigExists ? { tsconfig: tsConfigPath } : {}), | ||
| outDir: join(process.cwd(), 'dist'), | ||
| // sourcemap: optimized, | ||
| format: ['esm'], | ||
| target: 'chrome100', |
There was a problem hiding this comment.
@JReinhold you mentioned that we might want to add a FireFix version in here?
What do you think that version should be?
There was a problem hiding this comment.
Based on browserslist defaults and Can I Use usage table I'd suggest this:
| target: 'chrome100', | |
| target: ['chrome105', 'safari15', 'firefox105'], |
There was a problem hiding this comment.
FWIW, our migration guide indicates chrome 100+, if we change this, we would probably need to update that doc as well.
Issue:
What I did
How to test
If your answer is yes to any of these, please make sure to include it in your PR.