Skip to content

Conversation

@luboskmetko
Copy link
Member

This adds Performance class for performance optimization code. By default it adds async attribute to JS scripts in WP ( in particular wp-embed.js which is added by default by WP)

Copy link
Collaborator

@jakub300 jakub300 left a comment

Choose a reason for hiding this comment

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

Unfortunately life is not that simple,
many things will break with code like that. For example gravity forms relies on jQuery loaded synchronously and it has it's own script that must be loaded before DOMContentLoaded.

If you know that it's safe to load wp-embed asynchronously then please do but anything global has too many possible side effects.

@luboskmetko
Copy link
Member Author

@jakub300 thanks for pointing that out. Can you check this update version, I've tried to make it more generic.

Copy link
Collaborator

@jakub300 jakub300 left a comment

Choose a reason for hiding this comment

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

I like the new Idea. I would left things uncommented and scriptsToDefer as empty array. Also, considering the methods receive single script, name asyncScript/deferScript sounds better for me. Did not tested.

jakub300
jakub300 previously approved these changes Aug 31, 2017
@jakub300 jakub300 self-requested a review August 31, 2017 15:53
@jakub300
Copy link
Collaborator

Tested and seems to be working.

@luboskmetko
Copy link
Member Author

Thanks, I'll do those changes yet on Monday

@luboskmetko luboskmetko merged commit a068498 into xfiveco:master Sep 4, 2017
@luboskmetko luboskmetko deleted the performance-class branch September 4, 2017 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants