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(useTitle): prevent observe and titleTemplate been specified at the same time #2049

Merged
merged 8 commits into from Oct 16, 2022

Conversation

huynl-96
Copy link
Contributor

@huynl-96 huynl-96 commented Aug 5, 2022

Description

When setting observe to true, it only works well with default titleTemplate (which returns the exact input title e.g. '%s', (title) => title).

If changing titleTemplate into any other string template. e.g.

const title = useTitle('Vite', { observe: true, titleTemplate: '%s - %s' });

When we try to input the first character, title ref changes which then leads to the change of document.title. But since document.title and title are not the same due to titleTemplate, it again reassigns the new value from document.title to title ref.

The above process keeps repeating and freezes the entire page.

Screen.Recording.2022-08-05.at.16.51.38.mov

Additional context


  • Prevent using observe with titleTemplate
  • Improve types for useTitle
  • Replace all matches instead of only the first match for titleTemplate string
  • Add unit tests

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@antfu
Copy link
Member

antfu commented Aug 23, 2022

Maybe instead we should throw an error to now allow using observe with titleTemplate

@huynl-96
Copy link
Contributor Author

@antfu I've updated the implementation to throw error. Please kindly check it when you're available.
Thank you

@antfu antfu changed the title fix(useTitle): prevent title from update infinitely fix(useTitle): prevent observe and titleTemplate been specified at the same time Oct 16, 2022
@antfu
Copy link
Member

antfu commented Oct 16, 2022

In VueUse we don't throw errors as the message would increase the bundle size and impossible to do i18n. Changed to do it on type level, and force observe disabled when occurred at the same time.

@antfu antfu enabled auto-merge (squash) October 16, 2022 23:04
antfu
antfu previously approved these changes Oct 16, 2022
@antfu antfu merged commit 8c1ba50 into vueuse:main Oct 16, 2022
@huynl-96 huynl-96 deleted the fix/useTitle-types branch November 16, 2022 04:13
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

2 participants