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.

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

Do not prune *.d.ts files #66

Merged
merged 2 commits into from
May 12, 2020
Merged

Do not prune *.d.ts files #66

merged 2 commits into from
May 12, 2020

Conversation

dmattia
Copy link
Contributor

@dmattia dmattia commented May 4, 2020

Fixes #65

Files matching the regex "*.d.ts" will not be removed from node_modules.

Up until now, this library hasn't done two things: used regex anywhere or had exceptions to the default rules. This largely makes sense, as the current whitelisted dirs/files/extensions can be put into maps with O(1) lookup times.

This PR adds regex based exceptions, which cannot be put into a map, so the time complexity will be O(nm) where n is the number of regexes we whitelist as exceptions, and m is the time complexity to compute the regex.

I am going to argue that as long as the regex list is kept small (which I put in a comment in the code) that the practical time difference will be minimal.

Here is me running my forked tool over a 1.4GB monorepo:
Screen Shot 2020-05-04 at 11 22 33 AM

And here is a run over the same 1.4GB node_modules using the tool at HEAD:
Screen Shot 2020-05-04 at 11 29 28 AM

On repeated runs, they seem to execute in roughly the same time frame, so the performance impact of this PR seems negligible.

@tj
Copy link
Owner

tj commented May 6, 2020

Hmm tough call, these files aren't required at runtime are they? Just compile-time? My original plan was just to remove all stuff that is non-essential for deployments

@dmattia
Copy link
Contributor Author

dmattia commented May 6, 2020

Correct, they are just for compile time, so that makes sense!

What if I had the default list of ExceptionGlobs as empty, but let it be overwritten by a flag? So typescript folks could optionally do:

node-prune --exception-globs "*.d.ts,tsconfig.json" ./node_modules

Would that be preferrable?

@tj
Copy link
Owner

tj commented May 7, 2020

Yeah --include, --exclude flags might be nice, sounds fine to me!

@andreyvital
Copy link

@tj any way this getting released soon? We're having problems with the .d.ts right now :)

@dmattia
Copy link
Contributor Author

dmattia commented May 7, 2020

@tj Updated to add --include and --exclude flag options, both accepting regex strings, both defaulting to empty

@dmattia
Copy link
Contributor Author

dmattia commented May 7, 2020

Screen Shot 2020-05-07 at 9 53 45 AM

^ some examples of usage

@tj
Copy link
Owner

tj commented May 12, 2020

awesome thanks!

@tj tj merged commit 92b4625 into tj:master May 12, 2020
@KeKs0r
Copy link

KeKs0r commented Jul 28, 2020

This seems to be released, but when fetching the newest version there is no --exclude flag?

@tj
Copy link
Owner

tj commented Aug 3, 2020

looks ok to me!

   λ curl -sf https://gobinaries.com/tj/node-prune | sh

  ==> Downloading github.com/tj/node-prune@master
  ==> Resolved version master to v1.2.0
  ==> Downloading binary for darwin amd64
  ==> Installing node-prune to /usr/local/bin
  ==> Installation complete

   λ node-prune -h
Usage of node-prune:
  -exclude value
    	Glob of files that should not be pruned. Can be specified multiple times.
  -include value
    	Globs of files that should always be pruned in addition to the defaults. Can be specified multiple times.
  -verbose
    	Verbose log output.

Maybe you have another version installed elsewhere? (`which node-prune)

@KeKs0r
Copy link

KeKs0r commented Nov 6, 2020

Thanks a lot @tj I still had an old curl, which fetched the old version.

@jackpordi
Copy link

For anyone looking for a quick copy and paste:
node-prune --exclude "*.d.ts" -- exclude "tsconfig.json" ./node_modules

(Multiple --exclude flags are the key here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.d.ts file are deleted
5 participants