Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(lambda-nodejs): ensure same parcel version is used locally and in Docker #9874

Merged
merged 5 commits into from
Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-lambda-nodejs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,21 @@ same version will be used for installation. If a lock file is detected (`package
`yarn.lock`) it will be used along with the right installer (`npm` or `yarn`).

### Local bundling
If Parcel v2 is available it will be used to bundle your code in your environment. Otherwise,
If Parcel v2.0.0-beta.1 is available it will be used to bundle your code in your environment. Otherwise,
bundling will happen in a [Lambda compatible Docker container](https://hub.docker.com/r/amazon/aws-sam-cli-build-image-nodejs12.x).

For macOS the recommendend approach is to install Parcel as Docker volume performance is really poor.

Parcel v2 can be installed with:
Parcel v2.0.0-beta.1 can be installed with:

```bash
$ npm install --save-dev parcel@next
$ npm install --save-dev --save-exact parcel@2.0.0-beta.1
```

OR

```bash
$ yarn add --dev @parcel@next
$ yarn add --dev --exact parcel@2.0.0-beta.1
```

To force bundling in a Docker container, set the `forceDockerBundling` prop to `true`. This
Expand Down
12 changes: 9 additions & 3 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/bundlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { Runtime } from '@aws-cdk/aws-lambda';
import * as cdk from '@aws-cdk/core';
import { exec } from './util';

const PARCEL_VERSION = '2.0.0-beta.1';

interface BundlerProps {
relativeEntryPath: string;
cacheDir?: string;
Expand All @@ -31,14 +33,18 @@ export class LocalBundler implements cdk.ILocalBundling {
}
try {
const parcel = spawnSync(require.resolve('parcel'), ['--version']);
LocalBundler._runsLocally = /^2/.test(parcel.stdout.toString().trim()); // Cache result to avoid unnecessary spawns
const version = parcel.stdout.toString().trim();
LocalBundler._runsLocally = new RegExp(`^${PARCEL_VERSION}`).test(version); // Cache result to avoid unnecessary spawns
if (!LocalBundler._runsLocally) {
process.stderr.write(`Incorrect parcel version detected: ${version} <> ${PARCEL_VERSION}. Switching to Docker bundling.\n`);
}
return LocalBundler._runsLocally;
} catch {
return false;
}
}

private static _runsLocally?: boolean;
public static _runsLocally?: boolean; // public for testing purposes

constructor(private readonly props: LocalBundlerProps) {}

Expand Down Expand Up @@ -89,7 +95,7 @@ export class DockerBundler {
buildArgs: {
...props.buildArgs ?? {},
IMAGE: props.runtime.bundlingDockerImage.image,
PARCEL_VERSION: props.parcelVersion ?? '2.0.0-beta.1',
PARCEL_VERSION: props.parcelVersion ?? PARCEL_VERSION,
},
})
: cdk.BundlingDockerImage.fromRegistry('dummy'); // Do not build if we don't need to
Expand Down
33 changes: 33 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,36 @@ test('Local bundling', () => {
// Docker image is not built
expect(fromAssetMock).not.toHaveBeenCalled();
});

test('LocalBundler.runsLocally checks parcel version and caches results', () => {
LocalBundler._runsLocally = undefined;

const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({
status: 0,
stderr: Buffer.from('stderr'),
stdout: Buffer.from('2.0.0-beta.1'),
pid: 123,
output: ['stdout', 'stderr'],
signal: null,
});

expect(LocalBundler.runsLocally).toBe(true);
expect(LocalBundler.runsLocally).toBe(true);
expect(spawnSyncMock).toHaveBeenCalledTimes(1);
expect(spawnSyncMock).toHaveBeenCalledWith(expect.stringContaining('parcel'), ['--version']);
});

test('LocalBundler.runsLocally with incorrect parcel version', () => {
LocalBundler._runsLocally = undefined;

jest.spyOn(child_process, 'spawnSync').mockReturnValue({
status: 0,
stderr: Buffer.from('stderr'),
stdout: Buffer.from('3.5.1'),
pid: 123,
output: ['stdout', 'stderr'],
signal: null,
});

expect(LocalBundler.runsLocally).toBe(false);
});