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

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

Closed
telion2 opened this issue Oct 28, 2022 · 15 comments
Closed

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

telion2 opened this issue Oct 28, 2022 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@telion2
Copy link

telion2 commented Oct 28, 2022

See #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 #1319 however I have my doubts there.

@telion2 telion2 added the bug Something isn't working label Oct 28, 2022
@telion2 telion2 changed the title Scale Causes 10 Times more Requests when Calling a Website Scale Causes 10 Times more Requests when Loading a Website Oct 28, 2022
@telion2 telion2 changed the title Scale Causes 10 Times more Requests when Loading a Website Scale Causes 10 Times more Requests when Loading the Website Oct 28, 2022
@telion2 telion2 changed the title Scale Causes 10 Times more Requests when Loading the Website Scale Causes 10 Times more Requests when Loading the Page Oct 28, 2022
@telion2
Copy link
Author

telion2 commented Oct 28, 2022

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
Copy link
Collaborator

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: #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

@maomaoZH maomaoZH self-assigned this Oct 31, 2022
@telion2
Copy link
Author

telion2 commented Oct 31, 2022

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
Copy link
Author

telion2 commented Oct 31, 2022

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
Copy link
Author

telion2 commented Oct 31, 2022

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
Copy link
Collaborator

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
Copy link
Author

telion2 commented Oct 31, 2022

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
Copy link
Collaborator

Sure thing. Glad to help.

@telion2
Copy link
Author

telion2 commented Oct 31, 2022

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 #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:

  • 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. This would also enable you, to use the names of the components, without adding "defineCustomElementScale" before the name.
  • defineCustomElements() should only load what is necessary.

@acstll
Copy link
Collaborator

acstll commented Oct 31, 2022

Thank you @maomaoZH for the awesome support 🙏

and it seems, that it also resolved the memory Issue of #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
Copy link
Collaborator

acstll commented Nov 3, 2022

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:

  • the bundle includes ~250 JavaScript files
  • only 2 of these are loaded initially
  • 4 more files are loaded lazily (the actual footer and 3 dependencies including icons), for a total of 6

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
Copy link
Collaborator

maomaoZH commented Nov 9, 2022

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

@telion2
Copy link
Author

telion2 commented Nov 9, 2022

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
Copy link
Collaborator

acstll commented Nov 9, 2022

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.

  • chunk-vendors.{hash}.js — this is source code for Vue and such
  • app.{hash}.js — this is where webpack matches chunk file names with actual import paths and other stuff…
  • 2492.{hash}.js — this is <scale-app-footer>
  • the other 3… — othe components, mainly icons
// 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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants