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

fix(client): avoid updating existing head tags, close #1268 #1314

Merged
merged 13 commits into from
Dec 14, 2023
Merged

Conversation

Mister-Hope
Copy link
Member

@Mister-Hope Mister-Hope commented Apr 20, 2023

No description provided.

@Mister-Hope Mister-Hope linked an issue Apr 20, 2023 that may be closed by this pull request
@Mister-Hope Mister-Hope marked this pull request as draft April 21, 2023 04:35
@Mister-Hope
Copy link
Member Author

Mister-Hope commented Apr 21, 2023

@meteorlxy Need some help here, the id is recalculated many times, and I am thinking if we can add a new type called ResolvedHeadConfig besides HeadConfig while has the id attribute?

The changes need to be applied in @vuepress/shared. and it may affect pageHeadResolver and PageHead .

If you think it's fine for use to do this breaking change, I can update this pr.

Also I am thinking if we could drop support of boolean attribute value.

@jcwangxp
Copy link

@meteorlxy the js reload issue causes wired behavior, and we need this bug to be fixed, so could you help to review and get it fixed?

@Mister-Hope Mister-Hope requested a review from meteorlxy May 24, 2023 08:02
@meteorlxy
Copy link
Member

meteorlxy commented Jul 4, 2023

Some alternative improvement:

  • Make site data head config more stable - only updated according to route locale
  • Inject site data head config into html template
  • Resolve site data head config in node side.

@meteorlxy
Copy link
Member

ref: vuejs/vitepress#3017

@Mister-Hope Mister-Hope marked this pull request as ready for review November 20, 2023 09:32
@Mister-Hope
Copy link
Member Author

@meteorlxy Maybe a review of this pr?

@meteorlxy
Copy link
Member

@Mister-Hope e2e failed for some reason. Also meet the error when testing locally. Try cd e2e && pnpm e2e:dev

@Mister-Hope

This comment has been minimized.

@Mister-Hope Mister-Hope marked this pull request as draft December 11, 2023 05:06
@Mister-Hope Mister-Hope marked this pull request as ready for review December 11, 2023 05:19
@meteorlxy meteorlxy merged commit bfbab28 into main Dec 14, 2023
22 checks passed
@meteorlxy meteorlxy deleted the head-tag branch December 14, 2023 09:12
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.

[Bug report] Head resources are loaded again when switching pages
3 participants