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

[Bug] Tree-Shaking is not working properly #230

Closed
nev21 opened this issue Jan 4, 2024 · 2 comments
Closed

[Bug] Tree-Shaking is not working properly #230

nev21 opened this issue Jan 4, 2024 · 2 comments
Assignees
Labels
bug Something isn't working p1 Priority 1 published-npm tree-shaking
Milestone

Comments

@nev21
Copy link
Contributor

nev21 commented Jan 4, 2024

Describe the bug
Consuming the latest version of the library 0.10.2 results in the following code being left behind in @nevware21/ts-async

var arrIndexOf = _unwrapFunction(INDEX_OF, ArrProto);
    _unwrapFunction(LAST_INDEX_OF, ArrProto);

    var arrSlice = _unwrapFunction(SLICE, ArrProto);

    var fnCall = _unwrapInstFunction(CALL);

Where the LAST_INDEX_OF (which is not used), is not getting tree-shaken from the build

To Reproduce
Steps to reproduce the behavior:

  1. Clone '@nevware21/ts-async`
  2. Rebuild and review the dist/es5/amd/ts-async.js (actually most of the formats not just amd)
  3. See unreferenced functions / properties not getting tree-shaken out of the generated code

Expected behavior
The unused functions should get tree-shaken out of existence in the consuming modules

@nev21 nev21 added bug Something isn't working p1 Priority 1 tree-shaking labels Jan 4, 2024
@nev21 nev21 self-assigned this Jan 4, 2024
@nev21 nev21 added this to the 0.10.3 milestone Jan 6, 2024
@nev21 nev21 closed this as completed Jan 8, 2024
@MSNev MSNev reopened this Jan 9, 2024
@MSNev
Copy link
Contributor

MSNev commented Jan 9, 2024

Issue is still occurring for some "other" usages, specifically in ApplicationInsights for the strSymSplit function

var strSplit = _unwrapFunction("split", StrProto);
hasSymbol() ? _unwrapFunction("split", StrProto) : polyStrSymSplit;

Trialing some local "fixes", it's possible to suppress this further by tagging the hasSymbol with the /*#__NO_SIDE_EFFECTS__*/ so that rollup knows that when it's not referenced it can be dropped.

@MSNev
Copy link
Contributor

MSNev commented Jan 9, 2024

Please add this to all of the "accessor" functions like getXXX(), hasXXX() when they don't have any side-effects as the package.json sideEffects: false doesn't always work.

@MSNev MSNev modified the milestones: 0.10.3, 0.10.4 Jan 9, 2024
nev21 added a commit that referenced this issue Jan 10, 2024
- By tagging isNode() as no side-effects (which is doesn't have) causes rollup to agressively remove it from a non-node build, which results in bundles that don't work for all environments.
  - So untagging
nev21 added a commit that referenced this issue Jan 10, 2024
…236)

- By tagging isNode() as no side-effects (which is doesn't have) causes rollup to agressively remove it from a non-node build, which results in bundles that don't work for all environments.
  - So untagging
@nev21 nev21 closed this as completed Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1 Priority 1 published-npm tree-shaking
Projects
None yet
Development

No branches or pull requests

2 participants