Odd file duplication in zip artifact after version 1.10.0 with individual packaging

[clawsl]

Hi,

First of all, thank you for the great plugin. It speeds up deployments for me a lot.

Recently I ran into an issue odd issue after trying to upgrade to a later version of it. What happens it the following:

• Up to 1.10.0 the zip files in the .serverless folder are fine and just contain a single bundled file for each function as expected. Working zip example:

• Starting with 1.10.1 the zips contain the original (unbundled) source file in addition to the bundled version. This seems to overwrite the bundled version during the cloud formation deployment for me. Broken zip example:

Configuration Info

serverless.yaml

package:
  individually: true

plugins:
  - serverless-esbuild
  - serverless-offline
  - serverless-offline-direct-lambda
  - serverless-reqvalidator-plugin
  - serverless-aws-documentation
  - serverless-pseudo-parameters

custom:
   esbuild:
     bundle: true
     minify: true
     packager: npm
     exclude: ['pg-native']
...

package.json

"devDependencies": {
    "dotenv": "^8.2.0",
    "eslint": "6",
    "eslint-config-airbnb-base": "^14.0.0",
    "eslint-config-node": "^4.0.0",
    "eslint-config-prettier": "^6.10.0",
    "eslint-plugin-import": "^2.20.1",
    "eslint-plugin-node": "^10.0.0",
    "jest": "^24.9.0",
    "prettier": "^1.19.1",
    "serverless": "^2.46.0",
    "serverless-aws-documentation": "^1.1.0",
    "serverless-esbuild": "^1.10.0",
    "serverless-offline": "^6.8.0",
    "serverless-offline-direct-lambda": "0.0.1",
    "serverless-pseudo-parameters": "^2.5.0",
    "serverless-reqvalidator-plugin": "^1.0.3",
    "typescript": "^3.6.3"
  },
  "dependencies": {
    "aws-sdk": "^2.656.0",
    "node-fetch": "^2.6.0",
    "node-gyp": "^7.1.2",
    "pg": "^8.3.0",
    "pg-copy-streams": "^5.1.1",
    "pump": "^3.0.0",
    "request": "^2",
    "rxjs": "^6.5.5",
    "s3-download-stream": "^1.1.1",
    "winston": "https://gitlab.com/mm123/winston-kf/-/archive/3.2.2/winston-kf-3.2.2.tar.gz",
    "xmlhttprequest": "^1.8.0"
  }

I tried several later versions (1.10.1 - the 1st version with the issue, 1.10.8, 1.11.0, 1.12.0) and all of those have the same problem for me on [email protected].

As soon as I downgrade to e.g. 1.10.0 or 1.9.1 via npm npm i [email protected] and run sls deploy the issue is gone and all works as expected . After that I upgrade and deploy again and the file duplication reappears for me.

The esbuild version npm installs is consistent (0.12.8) and the only thing that varies is the serverless-esbuild version.

[clawsl]

I traced the issue down to src/pack.ts in version 1.10.1

For debugging I changed the part handling individual files slightly

        // package each function
        yield Promise.all(buildResults.map(({ func, bundlePath }) => __awaiter(this, void 0, void 0, function* () {
            const name = func.name;
            console.log("func name: " + name);
            const excludedFiles = bundlePathList.filter(p => !bundlePath.startsWith(p));
            // allowed external dependencies in the final zip
            let depWhiteList = [];
            if (hasExternals) {
                const bundleDeps = helper_1.getDepsFromBundle(path.join(this.buildDirPath, bundlePath));
                const bundleExternals = ramda_1.intersection(bundleDeps, externals);
                depWhiteList = helper_1.flatDep(packagerDependenciesList.dependencies, bundleExternals);
            }
            const zipName = `${name}.zip`;
            const artifactPath = path.join(this.workDirPath, _1.SERVERLESS_FOLDER, zipName);
            // filter files
            var filesPathList = files.filter(({ localPath }) => {
                // exclude non individual files based on file path (and things that look derived, e.g. foo.js => foo.js.map)
                if (excludedFiles.find(p => localPath.startsWith(p)))
                    return false;
                // debug
                console.log("excl test: " + localPath + " startsWith " + `__only_${name}/`);
                // exclude files that belong to individual functions
                if (localPath.startsWith('__only_') && !localPath.startsWith(`__only_${name}/`))
                    return false;
                // exclude non whitelisted dependencies
                if (localPath.startsWith('node_modules')) {
                    // if no externals is set or if the provider is google, we do not need any files from node_modules
                    if (!hasExternals || isGoogleProvider)
                        return false;
                    if (
                    // this is needed for dependencies that maps to a path (like scoped ones)
                    !depWhiteList.find(dep => helper_1.doSharePath(localPath, 'node_modules/' + dep)))
                        return false;
                }
                return true;
            });
           // debug
            console.log("FPL before map: " + JSON.stringify(filesPathList));
            filesPathList = filesPathList
                // remove prefix from individual function extra files
                .map((_a) => {
                var { localPath } = _a, rest = __rest(_a, ["localPath"]);
                var retObj =  (Object.assign({ localPath: localPath.replace(`__only_${name}/`, '') }, rest));
                // console.log( "FP_map:" + JSON.stringify(_a) + "\n" +JSON.stringify(retObj));
                return retObj;
            });
           // debug
            console.log("FPL after map: " + JSON.stringify(filesPathList));
            const startZip = Date.now();
            yield utils_1.zip(artifactPath, filesPathList);
            const { size } = fs.statSync(artifactPath);
            this.serverless.cli.log(`Zip function: ${func.name} - ${utils_1.humanSize(size)} [${Date.now() - startZip} ms]`);
            // defined present zip as output artifact
            setFunctionArtifactPath.call(this, func, path.relative(this.serverless.config.servicePath, artifactPath));
        })));

[clawsl]

This results in the following output for my example function:

func name: sa-831-nissan-pkg-api-dev-pkg-auth
excl test: __only_sa-831-nissan-pkg-api-dev-adv-config startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-adv-config/pkg_adv_config.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-campaigns startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-campaigns/pkg_campaigns.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-details startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-details/pkg_package_details.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-perf-details startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-perf-details/pkg_package_perf_details.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-perf-history startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-perf-history/pkg_package_perf_history.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-perf-overview startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-perf-overview/pkg_package_perf_overview.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-unassigned-campaigns startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-package-unassigned-campaigns/pkg_unassigned_campaigns.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-packages startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-packages-export startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-packages-export/pkg_packages_export.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-packages/pkg_packages.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-pkg_config_scheduled_loader startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-pkg_config_scheduled_loader/index.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-pkg_config_scheduled_loader/main.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/

excl test: __only_sa-831-nissan-pkg-api-dev-pkg-auth startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: __only_sa-831-nissan-pkg-api-dev-pkg-auth/pkg_auth.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/
excl test: pkg_auth.js startsWith __only_sa-831-nissan-pkg-api-dev-pkg-auth/

FPL before map: [
  {"localPath":"__only_sa-831-nissan-pkg-api-dev-pkg-auth/pkg_auth.js","rootPath":"C:\\dev\\sa-831-nissan-pkg-api-ts\\.esbuild\\.build\\__only_sa-831-nissan-pkg-api-dev-pkg-auth\\pkg_auth.js"},
  {"localPath":"pkg_auth.js","rootPath":"C:\\dev\\sa-831-nissan-pkg-api-ts\\.esbuild\\.build\\pkg_auth.js"}]
FPL after map: [
  {"localPath":"pkg_auth.js","rootPath":"C:\\dev\\sa-831-nissan-pkg-api-ts\\.esbuild\\.build\\__only_sa-831-nissan-pkg-api-dev-pkg-auth\\pkg_auth.js"},
  {"localPath":"pkg_auth.js","rootPath":"C:\\dev\\sa-831-nissan-pkg-api-ts\\.esbuild\\.build\\pkg_auth.js"}]

The separated last 3 excl test lines are related to the problem and generate the duplicate file in the end after filtering; I tested which file contains the bundled version in the end and found out that removing the entry "localPath":"__only_sa-831-nissan-pkg-api-dev-pkg-auth/pkg_auth.js" ... fixed the issue for me.

This gives me the feeling the condition

                // exclude files that belong to individual functions
                if (localPath.startsWith('__only_') && !localPath.startsWith(`__only_${name}/`))
                    return false;

needs to be changed to cover my use case. Using just

                // exclude files that belong to individual functions
                if (localPath.startsWith('__only_'))
                    return false;

worked fine for me, but I'm not sure if there are cases where the 2nd condition is required.

[olup]

Hey, thank you very much for reporting an issue, and sorry for the delay in reviewing it. I can't seem to be able to reproduce it, would you have a minimal repository that would show us the bug ? There is indeed a reason for the second condition, so I'd like to experiment the bug myself to understand what in your setup exactly is causing it.

[yuejun92]

Hi @clawsl, I am also having the same issue currently even with the latest version 1.28.0. I am also using the individually package option. plugins:

• serverless-esbuild
• serverless-prune-plugin
• serverless-stack-termination-protection
• serverless-offline

package: individually: true

This case has been opened from for a year already, I wonder will it be fixed.

[samchungy]

Hi @clawsl, I am also having the same issue currently even with the latest version 1.28.0. I am also using the individually package option. plugins:

• serverless-esbuild
• serverless-prune-plugin
• serverless-stack-termination-protection
• serverless-offline

package: individually: true

This case has been opened from for a year already, I wonder will it be fixed.

We haven't been able to reproduce this so if you could give us a sample repo where this can be reproduced I'd be happy to take a look.

[yuejun92]

Hi @samchungy, test-esbuild.zip I have zipped up a stripped down version of my project. You can replicate it by the steps below:

1. go to the "serverless" folder and run command "npm install"
2. run command "npm run slsdeploy" (I have changed the command to "package" instead of "deploy")
3. go to the ".serverless" folder and get the zip that is packaged by serverless
4. run command "unzip -l vcInsuranceV1.zip"
5. You will be able to see the 2 insuance.js files inside.

I am using macOS.

Thank you.

[yuejun92]

Hi @floydspace, may I know if anyone is taking up this issue?

[samchungy]

Hey @yuejun92 my capacity is pretty limited right now. I'll try take a look this weekend or maybe some time after work. Thanks for the repo.

[yuejun92]

Hi @samchungy, thank you very much.

[samchungy]

I had a quick look and it's likely because your package pattern intersects the handler's. Could be an easy fix but I'll have a suss another time

[samchungy]

Here's what I think is happening:

• We tell esbuild to bundle ./vc-insuance/V1/insuance.handler.
• We copy it into your bundle at the same path ./vc-insurance/V1/insurance.js
• We look at your patterns and copy them over so the og one gets included too.

Is there any reason why you want to bundle everything in that folder?

[yuejun92]

Hi @samchung, I not sure what you mean by bundle everything in that folder. How should i change the folder structure or option to avoid this? Because I normally deploy this structure to AWS lambda and its working fine.

[samchungy]

So the way that esbuild works with bundle: true is that it will bring all the files it needs into itself into one entrypoint you specify. In this case it is ./vc-insuance/V1/insuance.handler. So you don't need to specify a package pattern. Esbuild will bundle all the code you need into the single file which will be named ./vc-insuance/V1/insuance.js based on the handler you specified.

When you specify ./vc-insuance/V1/** you are bringing in the raw insurance.js file again and that's why there is duplication. I'm not sure why you would also want to bring in the tests into your bundle. I'm not sure what your usecase is but typically you want to keep your lambdas as lean as possible to upload and optimise start times.

[yuejun92]

Hi @samchungy, after i remove the package from the function, it's working now. I wonder if the plugin should ignore same file name or ignore the package all together since it will be bundling all imports/require into a single file. Thank you very much for your help!

[samchungy]

Hi @samchungy, after i remove the package from the function, it's working now. I wonder if the plugin should ignore same file name or ignore the package all together since it will be bundling all imports/require into a single file. Thank you very much for your help!

It won't import the file if you have a file you dynamically read. Eg using fs.readfile so that's why it exists.