Initial ajax request not shown

fruitcake / laravel-telescope-toolbar

A toolbar for Laravel Telescope, based on the Symfony Web Profiler.

MIT License

778 stars 39 forks source link

Initial ajax request not shown #4

Closed arjanwestdorp closed 5 years ago

arjanwestdorp commented 5 years ago

My application loads an app.js file that does an axios request for it's data just before the </body> tag. This initial ajax request is not being shown in the toolbar. Figured it's because the toolbar is loaded after the app.js file. When changing the toolbar to load just before the </head> the initial ajax request is being show.

I guess there are reasons to not load the toolbar in the head section? I've tried loading it based on:  just before loading the app.js file and that works. Would it be an idea to make the place where the toolbar is injected configurable in Toolbar@injectToolbar?

If you like the idea I can create a PR for it.

barryvdh commented 5 years ago

Not sure what the downside is of having it in the head always (in development). I think that's discussed here initially: https://github.com/symfony/symfony/pull/8896#issuecomment-23603019 Not sure why it changed.

barryvdh commented 5 years ago

Debugbar also loads similar to this, but haven't heard the issue before there (I think). It would be relatively easy to make this configurable (just search for </head> instead of </body>, but not sure about the impact (yet). I guess there would be good reasons that Symfony doesn't do that.

arjanwestdorp commented 5 years ago

I saw that debugbar was doing it the same way indeed, so I thought there might be a good reason for it. I saw this comment about the debugbar which seems to be related: https://github.com/barryvdh/laravel-debugbar/issues/843#issuecomment-430556477

At the Symfony community they seem to be careful with this because of injecting the debugbar accidentally in partial responses. As people suggested to add it after the <title> element as well. https://github.com/symfony/symfony/pull/27047 and https://github.com/symfony/symfony/pull/20343.

I can try to make a PR tonight which will allow to make it configurable.

barryvdh commented 5 years ago

Yeah but those use-cases where about missing </body> tags, which I can understand. But we inject it at the end anyways.

The biggest objection I have is that we currently inline the code which obfuscates the source code. I'll see if I can render the assets as external assets.

Do you have a specific library, eg. datatables? Or your custom code?

arjanwestdorp commented 5 years ago

That makes sense indeed as it will be a big chunk of code in the first part of the source code.

It's my custom code, a normal Laravel app with an app.js that has a vue component which loads records in the mounted method. When doing a console.log in the mounted of this component and in the base_js.blade.php as very first argument, the one in the mounted method is logged before the one in the base_js.

barryvdh commented 5 years ago

Can you test this branch? https://github.com/fruitcake/laravel-telescope-toolbar/pull/6

arjanwestdorp commented 5 years ago

I've tested this branch, the idea works, but I encountered the following issues:

The <body> tag can have attributes, like in our case <body id="someid">. Therefor it could not be resolved. Temporarily used an empty body tag.
The JS file did not have the $excluded_ajax_paths variable and it would not track any XHR requests. Passing the excluded_ajax_paths in the baseJs controller method solved it and indeed the requests are now being logged.

barryvdh commented 5 years ago

oh right. Hmm I tried </title> or </head> but I think the div needs to be in the body probably.

arjanwestdorp commented 5 years ago

In chrome it works, but it's indeed not correct html. Maybe we can add the div to the body with javascript?

barryvdh commented 5 years ago

I splitted the head and the widget, so the head can be just before </head> and the widget later.

arjanwestdorp commented 5 years ago

This works like a charm 👍

barryvdh commented 5 years ago

Cool, tested it and also seems to work for datatables etc. Moved the requeststack to the head to make sure previous requests are logged before the current one. Closing this.