Skip to content

refactor: vue update by hmr api#243

Merged
yyx990803 merged 8 commits into
masterfrom
vue-hmr
May 25, 2020
Merged

refactor: vue update by hmr api#243
yyx990803 merged 8 commits into
masterfrom
vue-hmr

Conversation

@underfin
Copy link
Copy Markdown
Member

@underfin underfin commented May 24, 2020

I do it because i think it is useful for let the logics with built-in vue process can be as an plugin.Well I think the hmr with vue module should be implement by hmr api, instead of use internal logic

Other

I find top boundary will update self when deps is changed.eg blew case.

hot.accpet('A',  'B', () => {} )

I think this bevaior should be handle by user self.vite don't do it because sometimes user don't hope A full-reload when B change.
If the user want to do this bevaior.they can do like this blew.

hot.accpet('A',  'B', () => {
   import('A') // re-import A
} )

Or better way. we can add hmr bubble for not accpet deps.And accpet deps should not bubble.Like this blew.

// A
import 'B'
import 'C'
hot.accpet('A',  'B', () => {} )

A should not update when B changed.A should update when C change.

Comment thread src/node/server/serverPluginHmr.ts
Comment thread test/test.js Outdated
).toEqual([
// dispose for both dep and self
`foo was: 2`,
// `foo was: 2`,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is actually an intentional fix because self-accepting callbacks should be called when a dep change bubbles up to the boundary. (e.g. #207)

Comment thread src/client/client.ts Outdated
if (Array.isArray(deps) && deps.includes(changedPath)) {
deps.forEach((dep) => modulesToUpdate.add(dep))
} else {
} else if (deps === changedPath) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Removal of the check is intentional. This should be reverted.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also let's avoid mixing unrelated changes in the same PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy that.
But I change it becasue any change will bubbles up to the boundary when i use hmr api for refactor vue file hmr.That will cause hmr don‘t minimize update for vue file.And also maybe the pr is unnessary.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I revert this hmr change already.we can just do it with current hmr api.

@yyx990803
Copy link
Copy Markdown
Member

I think something is going on with appveyor, the fails seem unrelated.

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