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

[4.4][bugfix] Use iife for the scripts core and validate #43755

Merged
merged 5 commits into from
Aug 1, 2024

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Jul 8, 2024

Pull Request for Issue #43714 .

Summary of Changes

Use iife instead of esm for the scripts core and validate

Testing Instructions

  • Apply the PR
  • run npm install
  • Check that authenticating in the administrator works
  • Create a new article, don't add anything and press save, the validation error should appear
  • still in the article edit view, open the browser console, and type const initialize = 1; and hit Enter.

Before: Error SyntaxError: Identifier 'initialize' has already been declared
After: No error

Actual result BEFORE applying this Pull Request

Leaking variables in the global scope

Expected result AFTER applying this Pull Request

NO leaking variables in the global scope

Link to documentations

Please select:

  • [] Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@Fedik

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev labels Jul 8, 2024
@dgrammatiko dgrammatiko changed the title [4.4][bugfix] Use life for the scripts core and validate [4.4][bugfix] Use iife for the scripts core and validate Jul 8, 2024
@Fedik
Copy link
Member

Fedik commented Jul 8, 2024

thanks! I will test it later

@Fedik Fedik added the bug label Jul 8, 2024
@Fedik

This comment was marked as outdated.

@Fedik Fedik linked an issue Jul 8, 2024 that may be closed by this pull request
@Fedik
Copy link
Member

Fedik commented Jul 8, 2024

No, Ignore me 😄

@Fedik
Copy link
Member

Fedik commented Jul 8, 2024

It works, but, wee need to set some globals back. please add:

window.JFormValidator = JFormValidator;
window.punycode = punycode;

Someone may use it in their scripts, even though punycode not officially shipped.
Maybe with some comment, that it will be removed in future.

@dgrammatiko
Copy link
Contributor Author

Classes exposed to the global scope

@Fedik
Copy link
Member

Fedik commented Jul 8, 2024

I have tested this item ✅ successfully on 348daed


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43755.

@Fedik
Copy link
Member

Fedik commented Jul 8, 2024

For testing.
Open Article editing, open browser console, and type const initialize = 1; and hit Enter.

Before:
Error SyntaxError: Identifier 'initialize' has already been declared
After:
No error;

@Quy
Copy link
Contributor

Quy commented Jul 17, 2024

I have tested this item ✅ successfully on 348daed


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43755.

@Quy
Copy link
Contributor

Quy commented Jul 17, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43755.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 17, 2024
@MacJoom MacJoom self-assigned this Aug 1, 2024
@MacJoom MacJoom added this to the Joomla! 4.4.7 milestone Aug 1, 2024
@MacJoom MacJoom merged commit 0959556 into joomla:4.4-dev Aug 1, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 1, 2024
@MacJoom
Copy link
Contributor

MacJoom commented Aug 1, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

validate.js throws Uncaught SyntaxError: redeclaration of const errors
5 participants