serverless-esbuild icon indicating copy to clipboard operation
serverless-esbuild copied to clipboard

Remove buildOptions access before initialization

Open Edweis opened this issue 1 year ago • 3 comments

Issue:

With the following settings in serverless.yml:

custom:
  esbuild:
    packager: pnpm
    target: es2019
    minify: true
    bundle: true

When running serverless offline, the offline server starts but when a file is changed, serverless-esbuild runs into an infinite loop and keeps compiling until the process is killed.

image

Investigation

After some investigation, I found that chikidar is initialized with

{
    "options": {
        "ignored": [
            null, // (1) 
            "dist",
            "node_modules",
            null, // (2)
            "node_modules/serverless-esbuild"
        ],
        "awaitWriteFinish": true,
        "ignoreInitial": true
    },
    "defaultPatterns": [
        "./**/*.(js|ts)"
    ]
}

Reading the code, (1)and (2) should actually be respectively .esbuild and .build since they are the default value from serverless-esbuild (link).

The issue is that getCachedOptions ran for the first time before this.outputWorkFolder and this.outputBuildFolder are defined. Hence the cache is populated with null instead.

Fix

This fix, makes this.outputWorkFolder and this.outputBuildFolder initialized with the context value instead of the one from getCachedOptions.

Edweis avatar Jul 19 '22 09:07 Edweis

Should be resolved by https://github.com/floydspace/serverless-esbuild/pull/337

samchungy avatar Jul 19 '22 12:07 samchungy

I'm having the same issue, but with 1.32.5 which should include https://github.com/floydspace/serverless-esbuild/pull/337

At the time getBuildOptions() runs, this.outputWorkFolder and this.outputBuildFolder are undefined, so this line https://github.com/floydspace/serverless-esbuild/blob/master/src/index.ts#L245 puts undefined into the watch.ignore array.

Then these two lines https://github.com/floydspace/serverless-esbuild/blob/master/src/index.ts#L137-L138 are looking for this.buildOptions.outputWorkFolder and this.buildOptions.outputBuildFolder, but those properties don't exist on this.buildOptions.

I'm not sure if this.outputWorkFolder and this.outputBuildFolder are supposed to come from somewhere outside this plugin (are they initialised by Serverless itself?), but certainly in my case they aren't defined.

Possibly this line https://github.com/floydspace/serverless-esbuild/blob/master/src/index.ts#L245 should look more like

ignore: [this.outputWorkFolder || WORK_FOLDER, 'dist', 'node_modules', this.outputBuildFolder || BUILD_FOLDER],

to make use of the constants?

dave-irvine avatar Aug 08 '22 09:08 dave-irvine

I'm having the same issue, but with 1.32.5 which should include #337

At the time getBuildOptions() runs, this.outputWorkFolder and this.outputBuildFolder are undefined, so this line https://github.com/floydspace/serverless-esbuild/blob/master/src/index.ts#L245 puts undefined into the watch.ignore array.

Then these two lines https://github.com/floydspace/serverless-esbuild/blob/master/src/index.ts#L137-L138 are looking for this.buildOptions.outputWorkFolder and this.buildOptions.outputBuildFolder, but those properties don't exist on this.buildOptions.

I'm not sure if this.outputWorkFolder and this.outputBuildFolder are supposed to come from somewhere outside this plugin (are they initialised by Serverless itself?), but certainly in my case they aren't defined.

Possibly this line https://github.com/floydspace/serverless-esbuild/blob/master/src/index.ts#L245 should look more like

ignore: [this.outputWorkFolder || WORK_FOLDER, 'dist', 'node_modules', this.outputBuildFolder || BUILD_FOLDER],

to make use of the constants?

ah yep you are right. Pre-#337 they were initialised in the constructor. Let me think about this and I'll put something up to fix it

samchungy avatar Aug 09 '22 00:08 samchungy