8000 [WebProfilerBundle] Split and refactor all the JavaScript code by javiereguiluz · Pull Request #50790 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[WebProfilerBundle] Split and refactor all the JavaScript code #50790

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2023
Merged

[WebProfilerBundle] Split and refactor all the JavaScript code #50790

merged 1 commit into from
Jun 30, 2023

Conversation

javiereguiluz
Copy link
Member
@javiereguiluz javiereguiluz commented Jun 27, 2023 •
edited
Loading
Q A
Branch? 6.4
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

As discussed in the past with @stof, the current JavaScript code is less than ideal because everything is coded into the same Sfjs object. That's why e.g. the toolbar loads code to handle tabs and togglers (Which don't exist in the database), the logger profiler panel, the mailer profiler panel, etc.

This PR splits the code into different files:

  • base_js.html.twig keeps the code that it's only used in most/all profiler panels
  • toolbar_js.html.twig now stores the code that it's only used in the toolbar (mostly about Ajax requests)
  • Logger, Mailer, Form, etc. panels now store the code that only applies to those panels

This feature is ready to review but:

  • In toolbar_js.html.twig there's a bug that I can't fix. I need your help. It says "Illegal invocation", which is related to the usage of this in the call: oldXHROpen.apply(this, Array.prototype.slice.call(arguments)); because this it's not the XHR object but the new SymfonyWebDebugToolbar class.
  • In toolbar_js.html.twig I want to refactor more code to use modern JS features after the previous bug is fixed.

Thanks!

@javiereguiluz javiereguiluz added this to the 6.4 milestone Jun 27, 2023
@javiereguiluz
Copy link
Member Author

@stof your review has made me change my mind about some part of this PR. Modernizing the code of the toolbar means that many things will break. Let's keep the old code that works perfectly.

This PR now only splits the toolbar code and modernizes the profiler code.

Thanks!

@javiereguiluz
Copy link
Member Author

Christophe, thanks for the new review. I've made all the changes that you proposed. Thanks!

@fabpot
Copy link
Member
fabpot commented Jun 30, 2023

Thank you @javiereguiluz.

@fabpot fabpot force-pushed the toolbar_modern_js branch from 13a2732 to 619bba1 Compare June 30, 2023 06:31
@fabpot fabpot merged commit 3e8566c into symfony:6.4 Jun 30, 2023
@javiereguiluz javiereguiluz deleted the toolbar_modern_js branch June 30, 2023 15:30
@wouterj wouterj mentioned this pull request Jul 2, 2023
Merged
nicolas-grekas added a commit that referenced this pull request Jul 3, 2023
@nicolas-grekas
minor #50849 [WebProfilerBundle] Fix minify test after JS refactor (w…
03e4b87 
…outerj)

This PR was merged into the 6.4 branch.

Discussion
----------

[WebProfilerBundle] Fix minify test after JS refactor

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This test makes sure the JS files are valid after we "minify" them by removing newlines. After the refactor of #50790, it was not updated correctly causing a red CI.

This fixes the test, and makes it more future proof.

Commits
-------

3caa17a [WebProfilerBundle] Improve minify test after JS refactor
@HypeMC HypeMC mentioned this pull request Dec 7, 2023
Merged
fabpot added a commit that referenced this pull request Dec 9, 2023
@fabpot
bug #52937 [WebProfilerBundle] Fix "Copy as cURL" (HypeMC)
f58cf04 
This PR was merged into the 6.4 branch.

Discussion
----------

[WebProfilerBundle] Fix "Copy as cURL"

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #52920
| License       | MIT

The bug was introduced in #50790.

#SymfonyHackday

Commits
-------

c97f0f9 [WebProfilerBundle] Fix Copy as Curl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof approved these changes

Assignees
No one assigned
Labels
Status: Reviewed WebProfilerBundle
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
4 participants
@javiereguiluz @fabpot @stof @carsonbot
0