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.

Sign up for GitHub

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

import named export from parcel/watcher #2341

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

import named export from parcel/watcher #2341

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b91883b
import named export from parcel/watcher
josefaidt Dec 14, 2024
4e10504
try injecting subscribe
josefaidt Dec 17, 2024
ce09114
Merge remote-tracking branch 'upstream/main' into parcel-watcher-use-…
josefaidt Dec 17, 2024
e396991
revise subscription mock
josefaidt Dec 17, 2024
4f0c274
Merge remote-tracking branch 'upstream/main' into parcel-watcher-use-…
josefaidt Dec 17, 2024
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
5 changes: 5 additions & 0 deletions .changeset/chilled-pants-kiss.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@aws-amplify/sandbox': patch
---

move from importing the default export from the commonjs distribution of parcle/watcher to named export
12 changes: 8 additions & 4 deletions packages/sandbox/src/file_watching_sandbox.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, beforeEach, describe, it, mock } from 'node:test';
import path from 'path';
import watcher from '@parcel/watcher';
import watcher, { subscribe } from '@parcel/watcher';
import {
CDK_DEFAULT_BOOTSTRAP_VERSION_PARAMETER_NAME,
FileWatchingSandbox,
Expand Down Expand Up @@ -39,7 +39,8 @@ import {

// Watcher mocks
const unsubscribeMockFn = mock.fn();
const subscribeMock = mock.method(watcher, 'subscribe', async () => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const subscribeMock = mock.fn(async (_dir, _effect, _options) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const subscribeMock = mock.fn(async (_dir, _effect, _options) => {
const subscribeMock = mock.fn(async () => {

that should be fine, if not then something like this:

amplify-backend/packages/client-config/src/client-config-writer/client_config_writer.test.ts

Line 27 in 1400e6f

writeFile: mock.fn<(path: string, content: string) => Promise<void>>(() =>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was to appease the checks like this one where we're checking that subscribe was called with a specific argument, https://github.com/aws-amplify/amplify-backend/pull/2341/files#diff-a3a9ef2ca41c1c9b876d1523afccb0fa5478ffd7ef185395e103467e10bd6484R967

I can pull in the parameters type to give the arguments a type. The underscore satisfies tsc but I also had to add the eslint-disable comment to appease no-unused-vars

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps

const subscribeMock = mock.fn(async (..._args: Parameters<typeof subscribe>) => {
  return { unsubscribe: unsubscribeMockFn };
});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the mock.fn<> satisfies both tsc and linter. Let's define parameterless stub AND provides typing for argument assertion.

The _dir or ..._args are not relevant for mock . this is for TSC - but that can be handled with <>

return { unsubscribe: unsubscribeMockFn };
});
const packageManagerControllerFactory = new PackageManagerControllerFactory(
Expand Down Expand Up @@ -147,7 +148,8 @@ void describe('Sandbox to check if region is bootstrapped', () => {
ssmClientMock,
functionsLogStreamerMock as unknown as LambdaFunctionLogStreamer,
printer as unknown as Printer,
openMock as never
openMock as never,
subscribeMock as never
);

ssmClientSendMock.mock.resetCalls();
Expand Down Expand Up @@ -1163,7 +1165,8 @@ const setupAndStartSandbox = async (
testData.ssmClient,
testData.functionsLogStreamer,
printer as unknown as Printer,
testData.open ?? _open
testData.open ?? _open,
testData.subscribe ?? subscribe
josefaidt marked this conversation as resolved.
Show resolved Hide resolved
);

await sandboxInstance.start(sandboxOptions);
Expand Down Expand Up @@ -1216,4 +1219,5 @@ type SandboxTestData = {
ssmClient: SSMClient;
functionsLogStreamer: LambdaFunctionLogStreamer;
open?: typeof _open;
subscribe?: typeof subscribe;
josefaidt marked this conversation as resolved.
Show resolved Hide resolved
};
9 changes: 5 additions & 4 deletions packages/sandbox/src/file_watching_sandbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import debounce from 'debounce-promise';
import parcelWatcher, { subscribe } from '@parcel/watcher';
import { subscribe as _subscribe } from '@parcel/watcher';
import { AmplifySandboxExecutor } from './sandbox_executor.js';
import {
BackendIdSandboxResolver,
Expand Down Expand Up @@ -66,7 +66,7 @@ export const getBootstrapUrl = (region: string) =>
* Runs a file watcher and deploys
*/
export class FileWatchingSandbox extends EventEmitter implements Sandbox {
private watcherSubscription: Awaited<ReturnType<typeof subscribe>>;
private watcherSubscription: Awaited<ReturnType<typeof _subscribe>>;
private outputFilesExcludedFromWatch = ['.amplify'];
private filesChangesTracker: FilesChangesTracker;

Expand All @@ -79,7 +79,8 @@ export class FileWatchingSandbox extends EventEmitter implements Sandbox {
private readonly ssmClient: SSMClient,
private readonly functionsLogStreamer: LambdaFunctionLogStreamer,
private readonly printer: Printer,
private readonly open = _open
private readonly open = _open,
private readonly subscribe = _subscribe
) {
process.once('SIGINT', () => void this.stop());
process.once('SIGTERM', () => void this.stop());
Expand Down Expand Up @@ -197,7 +198,7 @@ export class FileWatchingSandbox extends EventEmitter implements Sandbox {
});

if (watchForChanges) {
this.watcherSubscription = await parcelWatcher.subscribe(
this.watcherSubscription = await this.subscribe(
watchDir,
async (_, events) => {
// Log and track file changes.
Expand Down
Loading