chore: reduce bundle size #220
Conversation
|
@benmccann suggests |
|
|
|
@userquin I think |
|
don't merge yet, I'm going to include yoctocolors and cleanup |
|
There is no difference using
|
|
@userquin Thanks |
|
I'm going to use (Upps: 263 bytes bigger using |
userquin
requested review from
benmccann and
antfu
and removed request for
benmccann
src/detect.ts
Outdated
| @@ -75,7 +43,12 @@ export async function detect({ autoInstall, programmatic, cwd }: DetectOptions = | |||
| process.exit(1) | |||
| } | |||
|
|
|||
| await ezspawn(`npm i -g ${agent.split('@')[0]}${version ? `@${version}` : ''}`, { stdio: 'inherit', cwd }) | |||
| await x(`npm i -g ${agent.split('@')[0]}${version ? `@${version}` : ''}`, undefined, { | |||
There was a problem hiding this comment.
should we use arguments here (second argument in x call)?
There was a problem hiding this comment.
You should. Same as how child process works
x('npm', ['i', '-g', ...])
There was a problem hiding this comment.
Also keep in mind this can throw if somehow the command fails (like if npm doesn't exist)
Not sure how that was being dealt with before. Maybe it's fine as is and something catches it further up
src/runner.ts
Outdated
| const getCmd = (a: Agent) => agents.includes(a) ? getCommand(a, 'agent', ['-v']) : `${a} -v` | ||
| const getV = (a: string, o: EzSpawnOptions = {}) => ezspawn(getCmd(a as Agent), o).then(e => e.stdout).then(e => e.startsWith('v') ? e : `v${e}`) | ||
| const getCmd = (a: Agent) => AGENTS.includes(a) ? getCommand(a, 'agent', ['-v']) : `${a} -v` | ||
| const getV = (a: string, o: Partial<TinyExecOptions> = {}) => x(getCmd(a as Agent), undefined, o).then(e => e.stdout).then(e => e.startsWith('v') ? e : `v${e}`) |
There was a problem hiding this comment.
should we include in getVarguments here (second argument in x call)?
| @@ -140,5 +146,10 @@ export async function run(fn: Runner, args: string[], options: DetectOptions = { | |||
| return | |||
| } | |||
|
|
|||
| await ezspawn(command, { stdio: 'inherit', cwd }) | |||
| await x(command, undefined, { | |||
There was a problem hiding this comment.
should we use arguments here (second argument in x call)?
There was a problem hiding this comment.
It'll default to an empty array under the hood if you pass undefined like this. So should be fine
There was a problem hiding this comment.
Looks like tinyexec doesn't support command string mode, check #224
For example, check CI actions here https://github.com/unocss/unocss/actions/runs/10388099755/job/28762993666
Co-authored-by: Superchupu <53496941+SuperchupuDev@users.noreply.github.com>
|
You should stick to picocolors imo and open issues upstream if you think there's a limitation It has been adopted by most other large projects (and more to come). Think it is better to align than take yet another rehash of the same algorithm/implementation pico is also the fastest |
Description
This PR includes:
find-up, replaced withpackage-manager-detector(find-up 6 dependencies and package-manager-detector 0 dependencies)fast-globwithtinyglobby(fast-glob 17 dependencies and tinyglobby 2 dependencies)@jsdevtools/ez-spawnwithtinyexec(@jsdevtools/ez-spawn 9 dependencies and tinyexec 0 dependencies)kleurwithpicocolors(kleur 0 dependencies and picocolors 0 dependencies)Right nownibuild is failing.nitgz file from 107KB to 77KBWe should remove agent.ts module, not being used.(agentmodule re-exporting pm detector stuff)Linked Issues
Additional context