Remove compile/test time libraries from the requirements

dwilches commented 3 years ago

We added this library to our project and we got several unexpected dependencies added to node_modules.

Looking into what they were, we noticed some look like not needed for using your library, but only at compile time or development time.

For example, minimist is only used from here.

This is an incomplete list of the dependencies that were added to our node_modules while installing @ng-neat/until-destroy:

        "@dsherret/to-absolute-glob"
        "fs.realpath": "^1.0.0",
        "inflight": "^1.0.4",
        "inherits": "2",
        "minimatch": "^3.0.4",
        "once": "^1.3.0",
        "path-is-absolute": "^1.0.0"
        "@nodelib/fs.stat": "2.0.4",
        "run-parallel": "^1.1.9"
        "@nodelib/fs.scandir": "2.1.4",
        "fastq": "^1.6.0"
        "@dsherret/to-absolute-glob": "^2.0.2",
        "fast-glob": "^3.2.2",
        "fs-extra": "^9.0.0",
        "is-negated-glob": "^1.0.0",
        "multimatch": "^4.0.0",
        ...

Are all these 1st-party dependencies needed for your end-users to be able to use the library?

        "glob": "^7.1.6",
        "minimist": "1.2.5",
        "ts-morph": "^7.1.2",
        "tslib": "^2.0.0"

Thanks.

NetanelBasal commented 3 years ago

Users need this for the migration script.

dwilches commented 3 years ago

The migration is a one-time process. But after the migration is done, we keep dragging that dependency around.

Does it make sense to move the migration script to a second helper package?

Something like @.***/until-destroy-migration", which users can add, use, and then remove.

Thanks.

On Thu, Mar 18, 2021, 1:49 AM Netanel Basal @.***> wrote:

Users need this for the migration script https://github.com/ngneat/until-destroy#migration-from-view-engine-to-ivy .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ngneat/until-destroy/issues/151#issuecomment-801676583, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6KR2NXJOVEZ3XNP2FMZEDTEGO5XANCNFSM4ZLEMWVA .

sebastianrath commented 3 years ago

I'm running into the same issue at the moment. I'm using @ngneat/until-destroy inside an Electron app and ts-morph with it's 50 MB ends up in the final package but I don't see any purpose for it. @NetanelBasal Could you take a look at this issue again? Any help is highly appreciated!

NetanelBasal commented 3 years ago

You're welcome to submit a PR that separates it into a different package.

sebastianrath commented 3 years ago

You're welcome to submit a PR that separates it into a different package.

Perfect, I'll look into this :-)

bbarry commented 3 years ago

btw this version of ts-morph is introducing a vulnerability:

> npm audit --production 

                       === npm audit security report ===                        

# Run  npm update glob-parent --depth 5  to resolve 1 vulnerability

  Moderate        Regular expression denial of service                          

  Package         glob-parent                                                   

  Dependency of   @ngneat/until-destroy                                         

  Path            @ngneat/until-destroy > ts-morph > @ts-morph/common >         
                  fast-glob > glob-parent                                       

  More info       https://npmjs.com/advisories/1751                             

found 1 moderate severity vulnerability in 129 scanned packages
  run `npm audit fix` to fix 1 of them.

arturovt commented 3 years ago

Hey! I've published @ngneat/until-destroy@8.1.0 which doesn't depend on packages that are necessary for migration. The migration script is published as a separate package @ngneat/until-destroy-migration.

arturovt commented 3 years ago

Will close the issue for now, feel free to re-open if there're any issues.

ngneat / until-destroy

Remove compile/test time libraries from the requirements #151