Skip to content

feat: Remove tslib as a dependency #5331

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

Closed
wants to merge 1 commit into from
Closed

Conversation

AbhiPrasad
Copy link
Member

We no longer use tslib as a dependency on the SDKs.

Only place tslib is left is in angular - not sure if we can remove that. If we can, we'll have removed tslib as a top level dependency in the SDKs.

We no longer use tslib as a dependency on the SDKs.
@AbhiPrasad AbhiPrasad self-assigned this Jun 28, 2022
@Lms24
Copy link
Member

Lms24 commented Jun 29, 2022

I don't think we can remove tslib from the Angular SDK. ngc uses tsc and if I'm not mistaken, tsc needs tslib, right?

@AbhiPrasad
Copy link
Member Author

Closing this as no time atm, can follow up later.

@AbhiPrasad AbhiPrasad closed this Jun 30, 2022
@AbhiPrasad AbhiPrasad deleted the abhi-remove-tslib branch June 30, 2022 17:39
@AbhiPrasad
Copy link
Member Author

@lobsterkatie can we follow up with this? Would be nice to rm -rf the dep.

@SimenB
Copy link

SimenB commented Sep 15, 2022

If not removed, would it be possible to upgrade it to v2?

(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/utils/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/integrations/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/hub/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/core/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/browser/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/react/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(node:57375) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /Users/simen/repos/monorepo/node_modules/@sentry/tracing/node_modules/tslib/package.json.

@AbhiPrasad
Copy link
Member Author

I think this should be possible - let me ask around to get opinions from others.

@AbhiPrasad
Copy link
Member Author

Oh yeah, one thing about tslib v2 though is that the bundle size has gone up a little bit, which is why we were hesitant to upgrade: https://bundlephobia.com/package/[email protected]

@SimenB
Copy link

SimenB commented Sep 15, 2022

Hmm, fair point! However, I assume v2 is also present transitively in any bundle created, so that might not be a problem for actual bundles (i.e., it's already in the bundle)?

E.g. your own lockfile in this repo

[email protected]:
  version "2.0.1"
  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.0.1.tgz#410eb0d113e5b6356490eec749603725b021b43e"
  integrity sha512-SgIkNheinmEBgx1IUNirK0TUD4X9yjjBRTqqjggWCU3pUEqIk3/Uwl3yRixYKT6WjQuGiwDv4NomL3wqRCj+CQ==

tslib@^1.10.0, tslib@^1.8.1, tslib@^1.9.0, tslib@^1.9.3:
  version "1.14.1"
  resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00"
  integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==

tslib@^2.0.0, tslib@^2.0.3:
  version "2.1.0"
  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.1.0.tgz#da60860f1c2ecaa5703ab7d39bc05b6bf988b97a"
  integrity sha512-hcVC3wYEziELGGmEEXue7D75zbwIIVUMWAVbHItGPx0ziyXxrOMQx4rQEVEV45Ut/1IotuEvwqPopzIOkDMf0A==

tslib@^2.0.1, tslib@^2.1.0, tslib@^2.4.0:
  version "2.4.0"
  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.4.0.tgz#7cecaa7f073ce680a05847aa77be941098f36dc3"
  integrity sha512-d6xOpEDfsi2CZVlPQzGeux8XMwLT9hssAsaPYExaQMuYskwb+x1x7J371tWlbBdWHroy99KnVB6qIkUbs5X3UQ==

tslib@^2.3.0, tslib@^2.3.1:
  version "2.3.1"
  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.3.1.tgz#e8a335add5ceae51aa261d32a490158ef042ef01"
  integrity sha512-77EbyPPpMz+FRFRuAFlWMtmgUWGe9UOG2Z25NqCwiIjRhOf5iKGuzSe5P2w1laq+FkRy4p+PCuVkJSGkzTEKVw==

(you might wanna run yarn-deduplicate to at least only have 2.0.1 and 2.4.0 😀)


Of course, removing it entirely would be best if it's not even needed 👍

@AbhiPrasad
Copy link
Member Author

the lock file stuff is from dev dependencies (lerna monorepo stuffing everything in :P), but I might be wrong, let me investigate! Last yarn dedup run was #3798, we've reached the time for another one probably!

@SimenB
Copy link

SimenB commented Sep 15, 2022

Right, my point was more to show how prevalent the module (also in its v2 form) is, so bundle size might not be affected as most likely both v1 and v2 are already present

@lobsterkatie
Copy link
Member

@lobsterkatie can we follow up with this? Would be nice to rm -rf the dep.

Done! #5753

@lobsterkatie
Copy link
Member

the lock file stuff is from dev dependencies (lerna monorepo stuffing everything in :P), but I might be wrong, let me investigate! Last yarn dedup run was #3798, we've reached the time for another one probably!

And from the yarn-deduplicate README:

This package only works with Yarn v1. Yarn v2 supports package deduplication natively!

I think we should just finally do the upgrade to yarn 2. I started looking into it a while back, but of course got pulled into something else. Will put it back on my mental radar.

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.

4 participants