feat(flutter-packages-builder): create Flutter monorepo tool#84
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Node-based CLI tool (flutter-mono) intended to run tasks across a Flutter/Dart monorepo while selecting the Flutter SDK version per package (via FVM), driven by a repo-root flutter_mono.yaml config.
Changes:
- Introduces
tool/flutter-mono.mjsCLI withlist,run, andexeccommands. - Implements package discovery (via pubspec scanning), basic filtering, and per-package Flutter version resolution from
environment.flutter/.fvmrc. - Adds a minimal YAML parser to read
flutter_mono.yamlwithout external dependencies.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const result = spawnSync(command, { | ||
| cwd: pkg.dir, | ||
| stdio: 'inherit', | ||
| shell: true, | ||
| env: { |
There was a problem hiding this comment.
runInPackage uses spawnSync(command, { shell: true }) with a single string command. This makes execution dependent on the platform shell and re-parses the string (leading to subtle quoting bugs and inconsistent behavior vs other repo tooling like tool/build-flutter-packages.mjs, which uses spawnSync(file, args)). Consider switching to spawnSync(file, args, { shell: false }) (or a structured command representation in config) so commands run deterministically and arguments are not re-interpreted by the shell.
| for (const pattern of config.packages) { | ||
| const baseDir = pattern.replace(/\/{0,1}\*+$/, '').replace(/\/$/, '') || '.'; | ||
| scanForPubspecs(join(ROOT_DIR, baseDir), found); | ||
| } |
There was a problem hiding this comment.
discoverPackageDirs documents config.packages as “glob patterns”, but the implementation effectively only supports patterns that end with */** by stripping trailing wildcards and then scanning that base directory. If a user provides a glob like packages/*/subpkg, the * remains in the path and discovery will fail/miss packages. Either implement real glob expansion (e.g. using a glob library) or tighten the config contract/docs to “root directories to scan” rather than globs.
| scope: flags.scope, | ||
| flutter: flags.flutter || scriptFilter.flutter === true, | ||
| dart: flags.dart || scriptFilter.dart === true, | ||
| dirExists: flags['dir-exists'] ?? scriptFilter.dirExists ?? null, |
There was a problem hiding this comment.
Script filter keys are mixed between kebab-case and camelCase: CLI uses --dir-exists, but script-level config is read from scriptFilter.dirExists. Since parseYaml preserves keys as-is, a natural YAML key like dir-exists: (matching the CLI) would be ignored. Consider supporting both spellings or standardizing/documenting one naming convention for filter keys in flutter_mono.yaml.
| dirExists: flags['dir-exists'] ?? scriptFilter.dirExists ?? null, | |
| dirExists: flags['dir-exists'] ?? scriptFilter.dirExists ?? scriptFilter['dir-exists'] ?? null, |
| const command = extra.length > 0 ? `${scriptConfig.run} ${extra.join(' ')}` : scriptConfig.run; | ||
|
|
There was a problem hiding this comment.
runScript builds the command by concatenating extra into a single string. Because process.argv has already stripped shell quotes, extra.join(' ') loses argument boundaries/quoting (e.g. -- foo "a b" becomes foo a b) and can change the meaning of commands. Consider executing with spawnSync(file, args, ...) (no string join) or using a proper argv/shell-quoting strategy so arguments with spaces are preserved.
| } | ||
|
|
||
| case 'exec': | ||
| execCommand(extra.join(' '), packages, installedVersions, fallbackVersion, flags); |
There was a problem hiding this comment.
exec passes extra.join(' ') into execCommand, which has the same quoting/argument-boundary problem as run (arguments containing spaces will be split when the shell re-parses the string). Prefer passing the raw extra array through and invoking spawnSync(cmd, args, ...) to preserve the original argv exactly.
| execCommand(extra.join(' '), packages, installedVersions, fallbackVersion, flags); | |
| execCommand(extra, packages, installedVersions, fallbackVersion, flags); |
Details
Create Flutter monorepo tool