Skip to content

Commit

Permalink
fix(amplify-cli-core): use build script properly for overrides (#14093)
Browse files Browse the repository at this point in the history
* fix(amplify-cli-core): use build script properly for overrides

Updated the TypeScript compilation of overrides so that it doesn't require `node_modules/.bin/tsc`.
Instead, it simply relies on the `build` script to execute `tsc`.
This is more flexible and can support alternative setups w/ hoisting (e.g. via Yarn workspaces).

This is an override corollary fix to #11854, which is for custom resources.

This is an improvement over my previous PR in #13858 in that it works with any package manager
by ensuring the `--project` and `tsconfig.json` files are passed through to the `tsc` script.
The previous implementation didn't work with `npm` because it doesn't pass through additional
args like `yarn` does. The fix was easy: simply separate the build run with `--` so that the
remaining args are treated as positional for the `tsc` script.

I've confirmed the fix works with both `yarn` and `npm` 💪

#11889

* fix(amplify-cli-core): only include -- for npm

Tweaked the implementation slightly so that the `--` arg is only
passed for `npm` since it doesn't actually work with `yarn`.

#11889

* fix(amplify-cli-core): new packageManner runner

Added a new `runner` field to the PackageManager interface.
The key difference here is npm's runner is actually `npx`,
whereas yarn and pnpm just use the same executable as the runner.

Updated both the override and custom-resources to use the runner
to run tsc now instead of only looking in `node_modules`.

#11889

* fix: update api

---------

Co-authored-by: Kamil Sobol <[email protected]>
  • Loading branch information
brianlenz and sobolk authored Feb 3, 2025
1 parent 4d368b3 commit aab715d
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,8 @@ const buildResource = async (resource: ResourceMeta): Promise<void> => {
}
}

// get locally installed tsc executable

const localTscExecutablePath = path.join(targetDir, 'node_modules', '.bin', 'tsc');

if (!fs.existsSync(localTscExecutablePath)) {
throw new AmplifyError('MissingOverridesInstallationRequirementsError', {
message: 'TypeScript executable not found.',
resolution: 'Please add it as a dev-dependency in the package.json file for this resource.',
});
}

try {
execa.sync(localTscExecutablePath, {
execa.sync(packageManager.runner, ['tsc'], {
cwd: targetDir,
stdio: 'pipe',
encoding: 'utf-8',
Expand Down
2 changes: 2 additions & 0 deletions packages/amplify-cli-core/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,8 @@ export interface PackageManager {
// (undocumented)
readonly packageManager: PackageManagerType;
// (undocumented)
readonly runner: string;
// (undocumented)
version?: SemVer;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,7 @@ export const buildOverrideDir = async (cwd: string, destDirPath: string): Promis
const tsConfigSampleFilePath = path.join(__dirname, '..', '..', 'resources', 'overrides-resource', 'tsconfig.resource.json');
fs.writeFileSync(tsConfigDestFilePath, fs.readFileSync(tsConfigSampleFilePath));

// get locally installed tsc executable

const localTscExecutablePath = path.join(cwd, 'node_modules', '.bin', 'tsc');

if (!fs.existsSync(localTscExecutablePath)) {
throw new AmplifyError('MissingOverridesInstallationRequirementsError', {
message: 'TypeScript executable not found.',
resolution: 'Please add it as a dev-dependency in the package.json file for this resource.',
});
}
execa.sync(localTscExecutablePath, [`--project`, `${tsConfigDestFilePath}`], {
execa.sync(packageManager.runner, ['tsc', '--project', tsConfigDestFilePath], {
cwd: tsConfigDir,
stdio: 'pipe',
encoding: 'utf-8',
Expand Down
6 changes: 6 additions & 0 deletions packages/amplify-cli-core/src/utils/packageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface PackageManager {
readonly packageManager: PackageManagerType;
readonly lockFile: string;
readonly executable: string;
readonly runner: string;
readonly displayValue: string;
version?: SemVer;
getRunScriptArgs: (scriptName: string) => string[];
Expand All @@ -29,6 +30,7 @@ class NpmPackageManager implements PackageManager {
readonly packageManager = 'npm';
readonly displayValue = 'NPM';
readonly executable = 'npm';
readonly runner = 'npx';
readonly lockFile = 'package-lock.json';

getRunScriptArgs = (scriptName: string) => ['run-script', scriptName];
Expand All @@ -39,6 +41,7 @@ class YarnPackageManager implements PackageManager {
readonly packageManager: PackageManagerType = 'yarn';
readonly displayValue = 'Yarn';
readonly executable = 'yarn';
readonly runner = this.executable;
readonly lockFile = 'yarn.lock';
version?: SemVer;

Expand Down Expand Up @@ -66,6 +69,7 @@ class PnpmPackageManager implements PackageManager {
readonly packageManager: PackageManagerType = 'pnpm';
readonly displayValue = 'PNPM';
readonly executable = 'pnpm';
readonly runner = this.executable;
readonly lockFile = 'pnpm-lock.yaml';

getRunScriptArgs = (scriptName: string) => [scriptName];
Expand All @@ -77,11 +81,13 @@ class CustomPackageManager implements PackageManager {
readonly displayValue = 'Custom Build Command or Script Path';
lockFile;
executable;
runner;
version?: SemVer;

constructor() {
this.lockFile = '';
this.executable = '';
this.runner = '';
}
getRunScriptArgs = () => {
throw new AmplifyError('PackagingLambdaFunctionError', {
Expand Down

0 comments on commit aab715d

Please sign in to comment.