Skip to content

Commit

Permalink
fix(lambda): bundling fails with pnpm >= 8.4.0 (#26478) (#26479)
Browse files Browse the repository at this point in the history
Fix issue with order of `-f` flag and file path in `rm` command for `pnpm` esbuild bundling step to remove `node_modules/.modules.yaml` from output dir. This is continuing to cause bundling step to fail for `pnpm` >= 8.4.0 with no external `node_modules` specified per issue #26478.

Solved by moving the `-f` flag before file path in the `rm` command and updating relevant unit test. Please note that I haven't adjusted the `del` command for windows env as not sure if same issue occurs in that env.

Exemption Request: No changes to integration test output of `aws-lambda-nodejs/test/integ.dependencies-pnpm.js` and don't feel this warrants a separate integration test.

Closes #26478.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
damienhill committed Jul 24, 2023
1 parent 0531492 commit 1df243a
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
7 changes: 4 additions & 3 deletions packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export class Bundling implements cdk.BundlingOptions {
osCommand.copy(lockFilePath, pathJoin(options.outputDir, this.packageManager.lockFile)),
osCommand.changeDirectory(options.outputDir),
this.packageManager.installCommand.join(' '),
isPnpm ? osCommand.remove(pathJoin(options.outputDir, 'node_modules', '.modules.yaml')) + ' -f' : '', // Remove '.modules.yaml' file which changes on each deployment
isPnpm ? osCommand.remove(pathJoin(options.outputDir, 'node_modules', '.modules.yaml'), true) : '', // Remove '.modules.yaml' file which changes on each deployment
]);
}

Expand Down Expand Up @@ -349,12 +349,13 @@ class OsCommand {
return `cd "${dir}"`;
}

public remove(filePath: string): string {
public remove(filePath: string, force: boolean = false): string {
if (this.osPlatform === 'win32') {
return `del "${filePath}"`;
}

return `rm "${filePath}"`;
const opts = force ? ['-f'] : [];
return `rm ${opts.join(' ')} "${filePath}"`;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ test('Detects pnpm-lock.yaml', () => {
assetHashType: AssetHashType.OUTPUT,
bundling: expect.objectContaining({
command: expect.arrayContaining([
expect.stringMatching(/echo '' > "\/asset-output\/pnpm-workspace.yaml\".+pnpm-lock\.yaml.+pnpm install --config.node-linker=hoisted --config.package-import-method=clone-or-copy --no-prefer-frozen-lockfile && rm "\/asset-output\/node_modules\/.modules.yaml" -f/),
expect.stringMatching(/echo '' > "\/asset-output\/pnpm-workspace.yaml\".+pnpm-lock\.yaml.+pnpm install --config.node-linker=hoisted --config.package-import-method=clone-or-copy --no-prefer-frozen-lockfile && rm -f "\/asset-output\/node_modules\/.modules.yaml"/),
]),
}),
});
Expand Down

0 comments on commit 1df243a

Please sign in to comment.