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(plugin-vue): hmr not working when updating script+template at the same time with a template preprocessor #106

Merged
merged 2 commits into from Jul 15, 2023

Conversation

rashfael
Copy link
Contributor

Description

Fixes #28 and #76.

When using pug as a template preprocessor and if you update both script and template at the same time, you will get

Property "foo" was accessed during render but is not defined on instance.

as a browser console error instead of hmr updating properly.
This is because the template compilation relies on the compiled script being already in cache, which in this case it isn't, because the hmr requests both template and script modules at the same time (see #28 for timing).

This fix makes sure that the script cache is always populated before the template is compiled.

Additional context

I'm not sure if calling resolveScript on every template compile is overkill, but resolveScript has an early return if there is already cached result. I could not get the relevant context ("template+script update at same time") on compile time, since this information is lost when the hmr client makes two separate requests.


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 and follow the Commit Convention.
  • 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.

@zaxlct
Copy link

zaxlct commented Jun 6, 2023

I also encountered the same problem. Why hasn't this PR been merged yet.

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

This looks good to me

@patak-dev patak-dev requested a review from sodatea July 12, 2023 08:08
Copy link
Member

@sodatea sodatea left a comment

Choose a reason for hiding this comment

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

Thanks! It's a pleasure to review PRs with such detailed explanations.

@sodatea sodatea changed the title fix(plugin-vue): hmr not working when updating script+template at the same time with a template preprocessor (fix #28, #76) fix(plugin-vue): hmr not working when updating script+template at the same time with a template preprocessor Jul 15, 2023
@sodatea sodatea merged commit 93c444c into vitejs:main Jul 15, 2023
@rashfael rashfael deleted the template-preprocessor-hmr branch July 27, 2023 14:35
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.

HMR is fetching multiple updated modules triggered by the same change in parallel and not in series
4 participants