[IGNORE WIP] Use FP to decrease bundle size #4381

yordis · 2022-01-11T04:58:19Z

Note: I have no context of things, so take things with a grain of salt, I am just messing around in public.

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

If you've added code that should be tested, please add tests.
Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

yordis · 2022-01-11T05:00:13Z

packages/types/src/hub.ts

-  /**
-   * Checks if this hub's version is older than the given version.
-   *
-   * @param version A version number to compare to.
-   * @return True if the given version is newer; otherwise false.
-   *
-   * @hidden
-   */
-  isOlderThan(version: number): boolean;


It is not clear to me why such function would be in the Hub interface since nowhere in the codebase we call isOlderThan and only from inside the Hub class is where such function is defined, which leads me to believe that was added for testing purpose, but still, don't think it is needed.

Among with other functions, especially that the interface was marked as internal stuff

yordis · 2022-01-11T05:06:17Z

Hey @AbhiPrasad I am messing around with something, and I would like to get the output of size-limit job in the mid-time, would you mind running the jobs for the first time? 🙏🏻

…tion

yordis · 2022-01-11T09:40:35Z

Well, PoC proven (probably maybe, the correctness is wrong, but not that far off)

Changing the programming style to be more functional allows the minified to compress even more the code. Probably due to the fact that they cant minify things like method names and so on.

Also, probably easier to test, all you have are functions and data (hub data), at the cost of having to import functions instead using dot notations but I think it is beneficial still (bias)

Also, if you continue with this Hub could become just a type since all you care is a contract of the data structure behind, being said, meh.

Want to see how much I can improve it by doing the same strategy. Also really interesting to learn from doing this pattern some issues with missing higher-order functions that could be more specialized or some issue with classes exposing methods as protected but yet in the code, they are used as if they were public ...

4 AM, time to sleep ...

yordis · 2022-01-11T09:43:33Z

packages/browser/src/integrations/linkederrors.ts

@@ -48,7 +49,7 @@ export class LinkedErrors implements Integration {
   */
  public setupOnce(): void {
    addGlobalEventProcessor((event: Event, hint?: EventHint) => {
-      const self = getCurrentHub().getIntegration(LinkedErrors);
+      const self = getIntegration(getCurrentHub(), LinkedErrors);
      return self ? _handler(self._key, self._limit, event, hint) : event;


self is no longer this ... therefore, _key and _limit should be public ... thinking later how to continue to do more FP tuff in another part of the code base

yordis · 2022-01-11T09:44:28Z

packages/browser/src/sdk.ts

-  if (!hub.captureSession) {
-    return;
-  }
+  // TODO: Follow up on this


I get it but I dont ... probably need to circle back on this one

yordis · 2022-01-11T09:45:31Z

packages/core/src/integrations/inboundfilters.ts

        const clientOptions = client ? client.getOptions() : {};
        // This checks prevents most of the occurrences of the bug linked below:
        // https://github.com/getsentry/sentry-javascript/issues/2622
        // The bug is caused by multiple SDK instances, where one is minified and one is using non-mangled code.
        // Unfortunatelly we cannot fix it reliably (thus reserved property in rollup's terser config),
        // as we cannot force people using multiple instances in their apps to sync SDK versions.
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access


Same deal here, self is no longer this so such keys are public ....

yordis · 2022-01-11T09:45:59Z

packages/core/src/integrations/inboundfilters.ts

        return self._shouldDropEvent(event, options) ? null : event;
      }
      return event;
    });
  }

  /** JSDoc */
-  private _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
-    if (this._isSentryError(event, options)) {
+  public _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {


Moved all these functions since they could be static there fore, they could be a function outside the class

yordis · 2022-01-11T09:46:53Z

packages/hub/src/hub.ts

 */
-export class Hub implements HubInterface {
+export class Hub {


Circle back on removing the class altogether and make a simpler factory function.

yordis · 2022-01-11T09:48:11Z

packages/hub/src/hub.ts

+export function getStack(hub: Hub): Layer[] {
+  return hub.stack;
+}


A little trick, ultimately in an FP style, having function getters helps since only one-time the stack will be present since such key cant be minified type of thing, I think.

yordis · 2022-01-11T09:49:14Z

packages/hub/src/hub.ts

-      const registryHubTopStack = getHubFromCarrier(registry).getStackTop();
-      setHubOnCarrier(activeDomain, new Hub(registryHubTopStack.client, Scope.clone(registryHubTopStack.scope)));
+    if (!hasHubOnCarrier(activeDomain) || isOlderThan(getHubFromCarrier(activeDomain), API_VERSION)) {
+      const registryHubTopStack = getStackTop(getHubFromCarrier(registry));


Basically, moved all the methods outside the class, and pass hub as the first parameter.

yordis · 2022-01-11T09:50:33Z

packages/hub/src/scope.ts


  /** Session */
-  protected _session?: Session;
+  public _session?: Session;


Same deal here, static clone is static so weird that it was marked as protected since newScope down below is actually accessing the value as it they were public

yordis · 2022-01-11T09:51:24Z

packages/integrations/src/angular.ts

@@ -64,7 +65,7 @@ export class Angular implements Integration {
  /**
   * @inheritDoc
   */
-  public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
+  public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub | any): void {


Follow up on this, any should be remove, temp hack

packages/integrations/src/angular.ts

packages/node/src/sdk.ts

yordis · 2022-01-13T09:11:31Z

Interesting, @sentry/minimal is just @sentry/hub but using the global Hub. Not sure what would be the benefits over it, especially having spread out things and dealing with compilers.

yordis · 2022-01-13T11:00:21Z

I am back to 🍏 land 😄

After getting familiar with some parts of the codebase, I am biased. I think it simplifies some parts of the code, at the cost of importing functions 🤷🏻

I could probably continue applying some "improvements" in areas like Client and removing the need of minimal and move such code to hub since it is just a wrapper around it that uses the global instance of the Hub.

Also, keep removing a lot of magical strings that can't be minified that ended up calling functions. Since you don't need to serialize most abstractions, we are better off doing it the functional programming way as I did in part of the code already.

I haven't gotten into Integrations, but I think the same deal doesn't need to be an interface. a simple factory function should do it

function makeRewriteFrames(options: { root?: string; prefix?: string; iteratee?: StackFrameIteratee } = {}) {
  // ...

  return ['RewriteFrames', function RewriteFramesProcessor(...) {
    // ....
  }]
}

Something like that.

Or the concept of the integration could be removed as it is right now and be closer to how Sentry work and call them `EventProcessor's instead. Instead of hiding things from people, they can register the processors directly.

But, I have no context of where the Integration concept is heading. So far, a simple function would do the job and may simplify a few things and makes it friendly for minification.

github-actions · 2022-02-03T12:01:43Z

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

yordis · 2022-02-03T14:39:44Z

No 😭 I am gonna continue breaking things 😭

github-actions · 2022-03-01T12:01:31Z

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

yordis · 2022-03-01T19:20:48Z

kekBYE PR was fun thou!

AbhiPrasad · 2022-03-01T20:51:44Z

If you wanna chat further - recently just opened up #4660

yordis · 2022-07-01T16:57:41Z

Was an amazing opportunity to hang out with y'all, really fun. 👋🏻 PR

yordis added 2 commits January 10, 2022 23:16

IGNORE

7bce4cc

asd

f553fef

yordis commented Jan 11, 2022

View reviewed changes

feat: move functions out of the class

d7a49d7

yordis force-pushed the testing-fp-things branch from d6f1001 to d7a49d7 Compare January 11, 2022 05:01

yordis added 4 commits January 11, 2022 00:19

chore: move cloneScope to file context instead of being a static func…

2fbffc4

…tion

chore: move more functions from the object out

ac90228

chore: move more hub functions out of the prototype

4ac12dd

chore: move fns outside class

3feec27

yordis mentioned this pull request Jan 11, 2022

Improve SDK API #4272

Closed

yordis added 7 commits January 11, 2022 02:04

chore: fix compiler issues

cef8a95

chore: move all methods from Hub class

e2da41a

chore: rename field

8558cad

chore: move declaration to body

482696f

chore: move functions out

c25359c

fix: compiler issues

98ebe31

fix: compilation issues

06042d0

yordis commented Jan 11, 2022

View reviewed changes

packages/integrations/src/angular.ts Outdated Show resolved Hide resolved

yordis commented Jan 11, 2022

View reviewed changes

packages/node/src/sdk.ts Outdated Show resolved Hide resolved

yordis added 5 commits January 13, 2022 02:15

chore: fix issues

07abdbe

chore: fix test

eff9fd4

fix node client

51976cb

chore: change session flusher api and hub functions

7c7bee2

fix: node client

121a455

yordis added 7 commits January 13, 2022 04:13

fix: minimal package

8bb8391

fix: core

ee258de

fix: browser

64e9d6b

fix: react

a9204ed

fix: nextjs

31c6711

fix: tracing

d1370e7

fix: compilers

8929a9f

github-actions bot added the Status: Stale label Feb 3, 2022

smeubank added Type: Discussion Type: Improvement and removed Status: Stale labels Feb 8, 2022

yordis mentioned this pull request Feb 11, 2022

Sentry JavaScript v7 Major Release Plan #4240

Closed

github-actions bot added the Status: Stale label Mar 1, 2022

yordis mentioned this pull request Mar 31, 2022

feat: change api to be functional segmentio/analytics-next#391

Closed

yordis mentioned this pull request Apr 28, 2022

add ability to capture customer events before library init segmentio/analytics-next#436

Merged

yordis closed this Jul 1, 2022

yordis mentioned this pull request Mar 11, 2025

Support for finer grained tree shaking connectrpc/connect-es#296

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IGNORE WIP] Use FP to decrease bundle size #4381

[IGNORE WIP] Use FP to decrease bundle size #4381

yordis commented Jan 11, 2022 •

edited

Loading

yordis Jan 11, 2022 •

edited

Loading

yordis commented Jan 11, 2022

yordis commented Jan 11, 2022

yordis Jan 11, 2022

yordis Jan 11, 2022

yordis Jan 11, 2022

yordis Jan 11, 2022

yordis Jan 11, 2022 •

edited

Loading

yordis Jan 11, 2022

yordis Jan 11, 2022

yordis Jan 11, 2022

yordis Jan 11, 2022

yordis commented Jan 13, 2022

yordis commented Jan 13, 2022 •

edited

Loading

github-actions bot commented Feb 3, 2022

yordis commented Feb 3, 2022

github-actions bot commented Mar 1, 2022

yordis commented Mar 1, 2022 •

edited

Loading

AbhiPrasad commented Mar 1, 2022

yordis commented Jul 1, 2022

[IGNORE WIP] Use FP to decrease bundle size #4381

[IGNORE WIP] Use FP to decrease bundle size #4381

Conversation

yordis commented Jan 11, 2022 • edited Loading

Note: I have no context of things, so take things with a grain of salt, I am just messing around in public.

yordis Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

yordis commented Jan 11, 2022

yordis commented Jan 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yordis Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yordis commented Jan 13, 2022

yordis commented Jan 13, 2022 • edited Loading

github-actions bot commented Feb 3, 2022

yordis commented Feb 3, 2022

github-actions bot commented Mar 1, 2022

yordis commented Mar 1, 2022 • edited Loading

AbhiPrasad commented Mar 1, 2022

yordis commented Jul 1, 2022

yordis commented Jan 11, 2022 •

edited

Loading

yordis Jan 11, 2022 •

edited

Loading

yordis Jan 11, 2022 •

edited

Loading

yordis commented Jan 13, 2022 •

edited

Loading

yordis commented Mar 1, 2022 •

edited

Loading