DiederikvandenB / apollo-link-sentry

Apollo Link middleware which enriches SentryJS with GraphQL data
MIT License
123 stars 36 forks source link

Importing apollo-link-sentry package gives 350kb of unwanted and unused code. #441

Closed ZerdoX-x closed 11 months ago

ZerdoX-x commented 2 years ago

This is really strange bug I came up with, but here is the reproduction repo (it took me really long time to make): ~zerdox/apollo-link-sentry-441

UPD: Now you can reproduce issue using both rollup and webpack. Originally it was found using vite, so I left it as an option too.

So how to see the issue?

  1. Clone repo (git clone), install dependencies (npm i), build the project (npm run build:rollup)
  2. Search phrase Unexpected single quote character in whole project (including node_modules and other ingnored by default folders. I've included .vscode/settings.json for that)
  3. You can see that search results includes files dist-rollup/index.xxx.js (this is bundle chunk which will go to user's browser) and node_modules/graphql/language/lexer.js, which is somehow maybe dependency (and not devdep) for this project?

How I can confirm it's an issue with apollo-link-sentry?

  1. Simply comment src/apollo.js:4 which imports this package (and it hasn't even been used!)
  2. Build the project again (npm run build:rollup). Repeat the search in your IDE.
  3. Now you can see this unwanted piece of code is now not included in user's bundle.

This bug is really strange because I was trying to fight with it in my production project and this issue could be fixed by removing prettier from devDependencies (which I was using only for formatting my code through npm script). All this started because my pagespeed (lighthouse) rank was low and it was complaining about 80kb of unused code. I started eliminating unused code from user's bundle chunks and this is the first piece of unused code I stumble upon.

Is my reproduction enough? I hope this will be fixed ASAP because importing apollo-link-sentry gives ~350kb of unused code (you can check size of the file by using ls -lh ./dist-rollup/*

I would not be so confused about this if apollo-link-sentry package listed graphql as it's dependencies, but what I see is:

image

deepmerge, dot-prop, tslib, zen-observable are the only dependencies (which don't have any dependencies)

ZerdoX-x commented 2 years ago

Using chrome's code coverage here is how chunk changed after removing apollo-link-sentry from the project (before, after):

image
image

Reduction of total size ~230kb Reduction of unused size ~170kb

spawnia commented 2 years ago

Open for suggestions or pull requests to fix this, I do not plan to work on this myself.

Poky85 commented 1 year ago

In my project I don't have problem with so much code added by apollo-link-sentry. I think it's because:

1) most of dependencies has already existed in project (@apollo/client, zen-observable-ts, @sentry/browser and graphql) 2) Tree shaking works well (I use Parcel 2.x)

Anyway I suggest to drop some dependencies that seems unecessary to me. I can prepare PR when agreed. E.g.:

export function withDefaults(options: SentryLinkOptions): FullOptions {
  return {
    …defaultOptions,
    …options,
    attachBreadcrumbs: {
      …defaultOptions.attachBreadcrumbs
      …options?.attachBreadcrumbs
    }
  };
}
spawnia commented 1 year ago

I can prepare PR when agreed.

Please do, I am fine with releasing a new major version that breaks things. Consider https://github.com/angular/angular/blob/main/CONTRIBUTING.md#-commit-message-format.

github-actions[bot] commented 11 months ago

:tada: This issue has been resolved in version 3.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: