huksley commented 2 years ago

This is a Bug Report

Description

What went wrong?

I am using the puppeteer package, and it creates inside a .local-chromium folder during package installation.

This package and .local-chromium folder are being packaged together into the zip file which end up being 352Mb.

What did you expect should have happened?

Package puppeteer should not be present in the resulting zip node_modules/ folder.

What was the config you used?

See config and full reproducible example in https://github.com/huksley/puppeteerless/blob/main/serverless.yml

What stacktrace or error message from your provider did you see?

I am getting very slow upload speed and error that unpacked size exceeds 200Mb.

Additional Data

Node and NPM version you're using: NodeJS v16.16.0 NPM v8.11.0
Serverless-Webpack Version you're using: "serverless-webpack": "5.8.0"
Webpack version you're using: "webpack": "^5.72.1", "webpack-node-externals": "^3.0.0"
Serverless Framework Version you're using: "serverless": "^3.21.0"
Operating System: Mac OS X 12.5
Stack Trace (if available): N/A

Reproducing

> git clone https://github.com/huksley/puppeteerless
> cd puppeteerless
> npm install
> npm run package
> ls -lah .serverless
total 843384
drwxr-xr-x   8 ruslan  staff   256B Aug 11 01:03 .
drwxr-xr-x  19 ruslan  staff   608B Aug 11 01:03 ..
-rw-r--r--   1 ruslan  staff    50M Aug 11 01:03 chrome.zip
-rw-r--r--   1 ruslan  staff   2.2K Aug 11 01:03 cloudformation-template-create-stack.json
-rw-r--r--   1 ruslan  staff   9.5K Aug 11 01:03 cloudformation-template-update-stack.json
-rw-r--r--   1 ruslan  staff   8.9M Aug 11 01:03 fonts.zip
-rw-r--r--   1 ruslan  staff   352M Aug 11 01:03 puppeteerless.zip
-rw-r--r--   1 ruslan  staff    18K Aug 11 01:03 serverless-state.json

huksley commented 2 years ago

Hi, I tried following options (and all of them does not work)

excludeFiles

custom:
  webpack:
    excludeFiles: "**/.local-chromium/**"

forceExclude

custom:
  webpack:
    includeModules:
      forceExclude:
        - puppeteer

nodeExternals in webpack.config.js

externals: [
    nodeExternals({
      modulesFromFile: {
        exclude: ["puppeteer"]
      }
    })
  ]

noInstall: true

custom:
  webpack:
    packagerOptions:
      noInstall: true

this actually causes error

Error:
[OperationalError: ENOENT: no such file or directory, stat '/Users/ruslan/src/s/.webpack/dependencies/node_modules'] {
  cause: [Error: ENOENT: no such file or directory, stat '/Users/ruslan/src/s/.webpack/dependencies/node_modules'] {
    errno: -2,
    code: 'ENOENT',
    syscall: 'stat',
    path: '/Users/ruslan/src/s/.webpack/dependencies/node_modules'
  },
  isOperational: true,
  errno: -2,
  code: 'ENOENT',
  syscall: 'stat',
  path: '/Users/ruslan/src/s/.webpack/dependencies/node_modules'
}

vicary commented 2 years ago

The modulesFromFile.exclude option is quite counter-intuitive, their exclude actually means excluded from marking the package as external which in turns is included in the bundle. Also the option values are keys in your package.json which means dependencies, depDependencies, peerDependencies and the deprecated optionalDependencies.

This is also why, in newer versions of webpack, they renamed include to excludeFromBundle and exclude to includeInBundle.

Try moving your puppeteer to devDependencies and use the following config,

externals: [
  nodeExternals({
     modulesFromFile: {
      excludeFromBundle: ["devDependencies"]
    }
  })
]

huksley commented 2 years ago

@vicary Thanks.

The problem is not that I get it bundled into .js file but rather that it is included into a resulting ZIP file... How to avoid that?

It is used by https://github.com/ghostery/adblocker/tree/master/packages/adblocker-puppeteer as a peer dependency and only used in TypeScript and resulting CJS does not require("puppeteer") at all.

vicary commented 2 years ago

If I am getting it correctly, your puppeteer lives inside node_modules in your ZIP. Serverless core, by default, brings everything in your node_modules into the final archive.

Our package installer, i.e. the noInstall option, doesn't matter here because if your adblocker-puppeteer lives in your dependencies, its peerDependencies are in turn installed along with it.

You may try the followings,

Explicitly install puppeteer in your devDependencies, and make sure you do not set false to package.excludeDevDependencies. This may overrides the peerDependencies relation in adblocker-puppeteer and tricks Serverless core to exclude it in the packing stage.
Excluding it via package.patterns !node_modules/puppeteer/**.

andrew-ignatiev commented 2 years ago

@vicary it's not ideal to use package.patterns especially when dependencies have a lot of peerDependencies. No one want to review manually peerDependencies in a large number of their lambda functions. For example TypeORM from version 3 started to use ts-node as optional peerDependency which on its own has typescript peerDependency which is huge and now all of it in packaged zip so the size of lamba.zip file has been started to grow significantly and upload of lambda functions have been started to fail due to AWS lambda limits. So I would say it's critical issue which requires urgent fix.

Devs from serverless-esbuild seems fixed it already https://github.com/floydspace/serverless-esbuild/pull/113

j0k3r commented 2 years ago

Removing optional peer deps is already supported since 3 years now https://github.com/serverless-heaven/serverless-webpack/pull/542

andrew-ignatiev commented 2 years ago

But it doesn't work by some reason. In my example the TypeORM version 3 started to be packaged with ts-node and typescript in node_modules and command npm list typescript says it's due to ts-node which is optional peerDependency.

huksley commented 2 years ago

@j0k3r I am not using puppeteer directly, but through chrome-aws-lambda package: https://github.com/huksley/puppeteerless/blob/main/package.json

you think I should go to maintainers of chrome-aws-lambda and ask them to add peerDependenciesMeta ?

j0k3r commented 2 years ago

@huksley I guess so, like it's done in typeorm. Even better, you can submit yourself a PR (it's hacktoberfest!)

@andrew-ignatiev regarding the code, ts-node shouldn't be packaged because it's optional and defined in peerDependenciesMeta. None of your deps required ts-node?

andrew-ignatiev commented 2 years ago

@j0k3r in unzipped lambda folder I see the next npm list outputs:

npm list ts-node └─┬ typeorm@0.3.7 └── ts-node@10.9.1

npm list typescript └─┬ typeorm@0.3.7 └─┬ ts-node@10.9.1 └── typescript@4.8.4

I have checked package-lock.json and ts-node is optional peerDependency here. I don't see any other non optional peerDependencies or primary dependencies.

In package-loc.json in section dependencies:

"ts-node": {
      "version": "10.9.1",
      "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-10.9.1.tgz",
      "integrity": "sha512-NtVysVPkxxrwFGUUxGYhfux8k78pQB3JqYBXlLRZgdGUqTO5wU/UyHop5p70iEbGhB7q5KmiZiU0Y3KlJrScEw==",
      "optional": true,
      "peer": true,
      "requires": {
        "@cspotcode/source-map-support": "^0.8.0",
        "@tsconfig/node10": "^1.0.7",
        "@tsconfig/node12": "^1.0.7",
        "@tsconfig/node14": "^1.0.0",
        "@tsconfig/node16": "^1.0.2",
        "acorn": "^8.4.1",
        "acorn-walk": "^8.1.1",
        "arg": "^4.1.0",
        "create-require": "^1.1.0",
        "diff": "^4.0.1",
        "make-error": "^1.1.1",
        "v8-compile-cache-lib": "^3.0.1",
        "yn": "3.1.1"
      },
      "dependencies": {
        "acorn-walk": {
          "version": "8.2.0",
          "resolved": "https://registry.npmjs.org/acorn-walk/-/acorn-walk-8.2.0.tgz",
          "integrity": "sha512-k+iyHEuPgSw6SbuDpGQM+06HQUa04DZ3o+F6CSzXMvvI5KMvnaEqXe+YVe555R9nn6GPt404fos4wcgpw12SDA==",
          "optional": true,
          "peer": true
        }
      }
    },

with 
```
"optional": true,
  "peer": true,
  ```

andrew-ignatiev commented 2 years ago

@j0k3r @vicary I've created minimal reproducible repo. Could you please check?

git clone https://github.com/andrew-ignatiev/serverless-typeorm
cd serverless-typeorm
nvm use 14
npm ci
npm run sls:package -- -s dev and wait 10-20 minutes
cd .serverless
mkdir ./res
unzip http-v1.zip -d ./res
cd ./res
npm list typescript

huksley commented 2 years ago

Maybe we could have a flag which will treat all peer dependencies as optional if they do not appear in dependencies of the parent package? I allways thought this is a convention.

huksley commented 2 years ago

@vicary Answering your post on Aug 12 (sorry, I just noticed it) I done suggested changes on original reproducible repo and nothing changed.

git clone https://github.com/huksley/puppeteerless nvm install 16 nvm use 16 npm install npm run package zip -Tv .serverless/puppeteerless.zip | grep puppeteer

andrew-ignatiev commented 2 years ago

@j0k3r @vicary I've created extra branch with TypeORM v2 typeorm-2. The main branch has TypeORM v3. You can check that in typeorm-2 the size of http-v1.zip is 13 Mb vs 31 Mb in main branch (lambda zip limit is 50 Mb). It's because of ts-node and typescript optional peerDependency.

git clone https://github.com/andrew-ignatiev/serverless-typeorm
cd serverless-typeorm
git checkout typeorm-2
nvm use 14
npm ci
npm run sls:package -- -s dev and wait 10-20 minutes
cd .serverless
mkdir ./res
unzip http-v1.zip -d ./res
cd ./res
npm list ts-node OR npm list typescript will output empty

Could you confirm the optional peerDependency issue in serverless-webpack package ?

andrew-ignatiev commented 2 years ago

Temporary workaround for short term in serverless.yml:

....
plugins:
  - serverless-webpack
...
  - serverless-scriptable-plugin
...
  scriptable:
    # add custom hooks
    hooks:
      after:package:createDeploymentArtifacts: 'find ./.serverless -name "*.zip" -exec zip --delete {} "node_modules/ts-node/*" "node_modules/typescript/*" "node_modules/aws-sdk/*" \;'
...

huksley commented 2 years ago

I have the same problem now with typescript bundled. I wonder how one could debug it easily?

Now I do npm list -a --omit dev and when check dependency tree visually. In my current situation typescript are required by react-intl but both have it marked as optional:

 "peerDependenciesMeta": {
    "typescript": {
      "optional": true
    }
  }

Dependecy tree excerpt:

func
├─┬ react-intl@6.2.0
│ ├─┬ @formatjs/intl@2.5.0
│ │ └── typescript@4.7.4 deduped
│ └── typescript@4.7.4

huksley commented 2 years ago

I updated my project, but you need to remove hook proposed by @andrew-ignatiev in serverless.yml to show the bug

git clone https://github.com/huksley/puppeteerless cd puppeteerless npm install npm run package ls -lah .serverless

huksley commented 2 years ago

I think the better way is to trace all dependencies how NextJS does https://github.com/vercel/nft

huksley commented 1 year ago

Why is this marked as "Awaiting reply"? All replies and reproducible repo has been given.

collierrgbsitisfise commented 1 year ago

as issue is still open and still relevant, i'll left ref to own one(there is repo with straightforward steps how to reproduce it)

@huksley as workaround you can use packageOptions -> scripts in order to exclude redundant package from final _nodemodules

// serverless.ts file
.....
webpack: {
    packagerOptions: {
        scripts: ['#define custom script here which will delete redundant files/folders'],
    },
},
....

huksley commented 1 year ago

@collierrgbsitisfise thanks I already doing it and use esbuild to trace dependencies and it works pretty well

serverless-heaven / serverless-webpack

Unable to exclude package from being bundled #1218

This is a Bug Report

Description

What went wrong?

What did you expect should have happened?

What was the config you used?

What stacktrace or error message from your provider did you see?

Additional Data

Reproducing

excludeFiles

forceExclude

nodeExternals in webpack.config.js

noInstall: true