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

Run JS in strict mode, vendorize jquery.are-you-sure #26742

Closed
wants to merge 2 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Aug 26, 2023

Partial extract from #25940: Use BannerPlugin to inject a 'use strict'; statement into all chunks, resulting in all JS running in strict mode.

jquery.are-you-sure only works in sloppy mode, so I removed the global vars it introduced and vendorized the result.

Example output:

Screenshot 2023-08-26 at 12 47 58

Some dependency chunks already have that strict mode statement, resulting in a double statement. This can not easily be avoided with BannerPlugin, but it does not really hurt either.

Screenshot 2023-08-26 at 12 47 51

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 26, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 26, 2023
Comment on lines +6 to +10

var settings = $.extend(
{
'message' : 'You have unsaved changes!',
'dirtyClass' : 'dirty',
Copy link
Member

@puni9869 puni9869 Aug 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we modify var by let

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are hundrets of lint errors in that file. We could try fixing them at the risk of breaking it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense.

@silverwind
Copy link
Member Author

Maybe we should just use https://github.com/github/session-resume and get rid of ays altogether.

@silverwind
Copy link
Member Author

This is too much of a hack, will follow up with something better.

@silverwind silverwind closed this Aug 26, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants