embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
339 stars 137 forks source link

@embroider/router too much recursion error occurs in fire fox browser #1455

Open NagarajNN opened 1 year ago

NagarajNN commented 1 year ago

too much recursion error occurs while navigating to another routes stackTrace shows this @embroider/router/dist/index.ts file I can't able put break point on that, image original error image

void-mAlex commented 1 year ago

hello @NagarajNN a few things to check:

lastly a reproduction repo would be greatly appreciated

NagarajNN commented 1 year ago

@void-mAlex it's doesn't happen in normal ember router embroider router version - 2.0.0 occurs in navigating to another routes http://localhost:{port}/#/dashboards/1/{dynamic-segment} when i navigate to another dashboard [here only dynamic-segment will be changed ] it occurs

void-mAlex commented 1 year ago

are you able to create a minimal reproduction of what you're seeing, as that would greatly help in fixing this

NagarajNN commented 1 year ago

any other option other than reproduction repo, can you please suggest @void-mAlex i can't able to track error seems like firefox browser default recursion limit error.

void-mAlex commented 1 year ago

I would ask you to try and narrow down where this is coming from, is the dynamic segment properly url encoded? I know FF/safari is less error tolerant than chrome in that respect a reproduction of your route in a new repo/project set up as you have it will go a long way to figuring out what is causing that if it is or not, possible to reproduce it that way, it will narrow it down to something else but at least will tell us more info

jembezmamy commented 6 months ago

I am experiencing the same issue, here are my investigations:

Zrzut ekranu 2024-05-6 o 12 20 37

The return original(name) line is being called recursively 55 times for each getRoute call, and 1100 times per single transition. We see in our logs that for some users on Firefox this causes the too much recursion error.

From what I understand, the getRoute method is being extended in setupRouter (line 72). But setupRouter is called every time we use urlFor (e.g. when we render a link). So depending on the number of links rendered, a lot of unnecessary recursion could be created. What's more, while the user uses the the app and new links are rendered, more and more new layers of recurrency are added.

I think we could leverage isSetup to make sure the getRoute is extended only once:

setupRouter(...args: unknown[]) {
      // @ts-expect-error extending private method
      let isSetup = super.setupRouter(...args);
+     if (isSetup) {
          let microLib = (this as unknown as { _routerMicrolib: { getRoute: (name: string) => unknown } })._routerMicrolib;
          microLib.getRoute = this._handlerResolver(microLib.getRoute.bind(microLib));
+     }
      return isSetup;
}

Is it safe to extend it only once? Maybe it was meant to run multiple times?