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

Loading lunr.js is not critical for page rendering and can be deferred #1050

Merged
merged 3 commits into from
Jun 16, 2022

Conversation

fekete-robert
Copy link
Collaborator

No description provided.

@LisaFC
Copy link
Collaborator

LisaFC commented Jun 15, 2022

@tekezo do you want to take this one?

@tekezo
Copy link
Contributor

tekezo commented Jun 15, 2022

Thank you!
It's good to defer loading lunr.js.

I have one point of concern.
offline-search.js depends lunr.js, it requires lunr is ready when DOMContentLoaded.
If the async attribute is specified, this preparation may not be completed in time.
So, I think it's better to remove async attribute.

- <script
+ <script defer

@fekete-robert
Copy link
Collaborator Author

fekete-robert commented Jun 16, 2022 via email

@tekezo
Copy link
Contributor

tekezo commented Jun 16, 2022

LGTM 👍

@LisaFC
Copy link
Collaborator

LisaFC commented Jun 16, 2022

Thanks for this!

@LisaFC LisaFC merged commit f45c98f into google:main Jun 16, 2022
fekete-robert added a commit to fekete-robert/docsy that referenced this pull request Sep 13, 2022
google#1050)

* Loading lunr.js is not critical for page rendering and can be deferred

* Remove async to make sure lunr is available for offline-search.js
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.

None yet

3 participants