telekom / scale

Scale is the digital design system for Telekom products and experiences.
https://telekom.github.io/scale/
Other
375 stars 82 forks source link

Scale Causes 10 Times more Requests when Loading the Page #1327

Closed telion2 closed 2 years ago

telion2 commented 2 years ago

See https://github.com/telekom/scale/issues/1319 for Code and Context.

Current Behavior The UIs that use scale make up to 10 times more calls.

This screenshot shows 280 requests when simply loading the page: Screenshot from 2022-10-28 12-02-28

Compared to the ~27 requests that are usually called without scale: Screenshot from 2022-10-28 12-14-37

Here would be a sample Response of one of the requests: ScaleSampleRequest.txt

Expected Behavior I would accept an increase of ~10-20 requests but not ~255 more. The problem is that this causes DDOS Detection Systems to trigger. In particular the WAF (web application firewall) of Telekom. We build our vue apps with vue cli, so I would have expected for Scale to be part of the build and not loaded separately. Admittedly not the first load, but if you use multiple tabs on different web pages that use scale the chances are high that you get blocked as a normal user. It also could be the root cause of https://github.com/telekom/scale/issues/1319 however I have my doubts there.

telion2 commented 2 years ago

I forgot to mention: The number of requests isn't necessarily fixed. This means sometimes, the number of requests can be lower, but I don't know yet, on what it depends.

maomaoZH commented 2 years ago

Hi @telion2 ,

Thanks for reporting this issue. I see there was a discussion between you and @acstll . Starting from version 3.0.0-beta.114, we have mechanism that allow you to do the "code split" style, meaning you only import the components you need. see here: https://github.com/telekom/scale/pull/1202. In your case, you use only footer component atm, therefore, you can try to do as following:

import { defineCustomElementScaleAppFooter } from '@telekom/scale-components';
defineCustomElementScaleAppFooter() // only `<scale-app-footer>` is loaded

Please let me know if this can reduce the massive number of requests and help you to resolve your issue.

Regards, Maomao

telion2 commented 2 years ago

Hi, thank you for the reply. I will try that out.

Since both Issues blocked our Production Deployment, I had to remove Scale to solve our change freeze. Meaning now I need to reintegrate Scale. But don't worry, I will do that but I need a bit of time.

telion2 commented 2 years ago

Well, I was curious and I realized that I still had a branch with scale integrated. I initially though it would take more time. So I made a little test, but I soon run into this: Screenshot from 2022-10-31 11-12-38 Screenshot from 2022-10-31 11-26-06

=> defineCustomElementScaleAppFooter might be the wrong name. I couldn't find the correct name in the documentation though.

telion2 commented 2 years ago

I looked up "@telekom/scale-components/loader" and I got redirected to an index.d.ts with the following content: Path: node_modules/@telekom/scale-components/loader/index.d.ts

export * from '../dist/types/components';
export interface CustomElementsDefineOptions {
  exclude?: string[];
  resourcesUrl?: string;
  syncQueue?: boolean;
  jmp?: (c: Function) => any;
  raf?: (c: FrameRequestCallback) => number;
  ael?: (el: EventTarget, eventName: string, listener: EventListenerOrEventListenerObject, options: boolean | AddEventListenerOptions) => void;
  rel?: (el: EventTarget, eventName: string, listener: EventListenerOrEventListenerObject, options: boolean | AddEventListenerOptions) => void;
}
export declare function defineCustomElements(win?: Window, opts?: CustomElementsDefineOptions): Promise<void>;
export declare function applyPolyfills(): Promise<void>;

I think there is an import missing that imports functions like defineCustomElementScaleAppFooter.

Yes I also made sure to use @telekom/scale-components": "^3.0.0-beta.114"

maomaoZH commented 2 years ago

Hi @telion2 , please don't import from @telekom/scale-components/loader but directly from @telekom/scale-components. We have not. updated the documentation yet. This will come soon.

telion2 commented 2 years ago

Apologies, I missed that. I corrected it and it works. I still need to deploy it to our dev Environment, and then I can check its impact on the requests and memory. Thank you for the quick reply though.

maomaoZH commented 2 years ago

Sure thing. Glad to help.

telion2 commented 2 years ago

So, that fix of yours was genius. We are now down to ~27 Requests. Furthermore, I also made a performance test on our dev environment, and it seems, that it also resolved the memory Issue of https://github.com/telekom/scale/issues/1319.

We called the UI 105 Times in one moment, and the memory usage didn't exceed 110 mib. In other words, this performance test was much more resource-heavy than the ones before, and it performed a lot better. (Before we hat usages up to 250mib)

So now I am happy and I can recommend integrating Scale again, after the documentation update. (Otherwise, in the worst-case scenario, I need to ask for the name of each component)

I would like to point out 2 small things:

acstll commented 2 years ago

Thank you @maomaoZH for the awesome support 🙏

and it seems, that it also resolved the memory Issue of https://github.com/telekom/scale/issues/1319.

that would be actually great!

Regarding the 2 remarks:

Having to call defineCustomElementScaleAppFooter() or similar functions can be a bit awkward. Basically, if I import a component I would expect the function defineCustomElementScaleAppFooter() to be called during the import if possible.

I completely agree. I do not know the reason why Stencil went this route. Some libraries offer both, they export the class, so you could define it yourself, plus an extra import that already defines the component. Sadly, this is not an option at this very moment.

defineCustomElements() should only load what is necessary

this is how it actually works already, only the components used are loaded lazily… (see https://stencil.docschina.org/blog/how-lazy-loading-web-components-work), but for some reason on your environment (as well as on another project in which @maomaoZH works) all these way-too-many unexpected calls are happening. This I would like to further investigate…

Thanks again for taking the time to provide further feedback @telion2

acstll commented 2 years ago

I did some more testing and I would like to share the results.

In a Vue 2 app made with vue-cli (wepback), the production bundle outputs one JavaScript per component in the Scale library, that is roughly 250 files, including the ~200 icons.

The test app uses only the Scale Footer, as in #1319. It behaves as expected:

I deployed this test app to a static server and it behaves the same (see the screenshot below).

The only difference I can see with your setup, is that the requests in your case are initiated by "other" and not the app itself (please see the Initiator column in DevTools).

@maomaoZH, @telion2 what do you think that could be, in your production environments, causing all these files to load unnecessarily?

image
maomaoZH commented 2 years ago

Hi @acstll , the test you did above, did you call defineCustomElements() ? And it loads only 6 files?

telion2 commented 2 years ago

The scripts in your screenshot look similar to those of mine. In my case, they have different names in various environments. I have also seen them in this name format: 1.js ... 270.js

but sometimes they run under different names like the screenshots above.

Also note the number of files loaded, was not always the same. I had instances where it loaded fewer files.

acstll commented 2 years ago

Hi @acstll , the test you did above, did you call defineCustomElements() ? And it loads only 6 files?

Yep. That's the expected behavious. I'm pasting some code below.

// entire main.js file
import Vue from 'vue'
import App from './App.vue'

import '@telekom/scale-components/dist/scale-components/scale-components.css'
import { defineCustomElements } from '@telekom/scale-components/loader'

defineCustomElements()

Vue.config.productionTip = false

new Vue({
  render: h => h(App),
}).$mount('#app')
<!-- App.vue -->
<template>
  <div id="app">
    <img alt="Vue logo" src="./assets/logo.png">
    <HelloWorld msg="Welcome to Your Vue.js App"/>
    <footer class="brand-footer-bar" role="contentinfo">
      <div class="container-fixed">
        <scale-app-footer id="footer" copyright="© 2022 Telekom Deutschland GmbH" variant="minimal">
        </scale-app-footer>
      </div>
    </footer>
  </div>
</template>

<script>
// …
</script>
maomaoZH commented 2 years ago

It seems lots of requests are caused by vue prefetch. see more https://cli.vuejs.org/guide/html-and-static-assets.html#prefetch. When I disable prefetch in my project config file, the issue is gone.
In summary, i can either:
1 partially load the components with prefetch config 2 disable prefetch config and use defineCustomElements() method