-
Notifications
You must be signed in to change notification settings - Fork 72
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
Replace broken Svelte 2 HMR with the one from rixo #156
Merged
Merged
Changes from all commits
Commits
Show all changes
89 commits
Select commit
Hold shift + click to select a range
45f83ef
remove hard dep on svelte
rixo 23fa354
style: remove trailing space
rixo eda88c6
extract makeHot from index.js
rixo 5b568c8
add support for HMR with svelte 3
rixo dcf722e
Merge remote-tracking branch 'upstream/master' into hmr
rixo 9dd5eec
add support for HMR with svelte-native
rixo 66b3061
introduce cmp.$$.replace instead of capture_state & inject_state
rixo 87b4192
better name for $$.replace
rixo 76ec3b8
fix restore props
rixo c4d9f09
style: autofix
rixo 21e0276
update native support
halfnelson 733852f
Merge pull request #3 from halfnelson/hmr-native-fix
rixo b2c1d29
remove global polution, now that it isn't needed anymore
rixo 81cfdd2
style: remove trailing space
rixo 189a246
refactor: extract svelte resolution to its own module
rixo 74636fb
resolve svelte from application entrypoint - fixes #109
rixo 0089526
add support for overriding svelte location by env variable
rixo e106329
Merge branch 'fix-link'
rixo 7787618
refactor: move makeHot to resolveSvelte (because it depends on svelte…
rixo 46db2fd
Merge branch 'hmr' into hmr-next
rixo 3122272
filter removed props using compilation infos
rixo 5fbd73b
fix hmr support for context
rixo 75bc4a3
better debug name for file with multiple extensions
rixo 638aa78
Merge branch 'hmr' into hmr-next
rixo 67e98a8
fix target / anchor in onMount when rendered to different ones (native)
rixo 9026759
fix support for native, & TNS 6
rixo e69f405
backport fixes from rollup plugin
rixo d6e4a1a
mock $capture_state to support preserving local state
rixo 91cfef7
respect noPreserveState option
rixo 0e5b209
implement full reload on failure
rixo 4c322c2
simpler error recuperation (not broken also)
rixo 8d5701a
exclude store subscriptions from preserved local state
rixo 1ae9f83
use svelte-hmr
rixo 3f54b18
add missing dep
rixo fadd9b4
upgrade to last svelte-hmr, with error management
rixo 4e6f38a
use last svelte-hmr
rixo 765d64d
fix svelte-hmr incorrectly registered as a dev dep
rixo 2204a8f
upgrade to last version of svelte-hmr
rixo dfbdb6f
cleaner hot-api adapter (also, fix svhs tests)
rixo 251588e
run accept handlers serially to prevent race conditions
rixo 3bc4671
add info message when svelte loc is overriden by env
rixo 3278148
fix async accept handlers race condition
rixo 2be1bd7
add webpack specific HMR done message
rixo 4a20c99
adapt for next version of svelte-hmr
rixo ed4e5ed
bump svelte-hmr
rixo e715674
make README hot
rixo 66fe7ac
fix tests
rixo 7929ed5
v0.0.1-0
rixo 73b11bb
v0.0.1-1
rixo 40068b8
fix links
rixo 1059907
v0.1.0
rixo 962a0be
Update README.md
rixo ada239e
better warning
rixo bdb5c58
bump svelte-hmr for svelte 3.16+ support
rixo 6caf00d
Merge branch 'master' of github.com:rixo/svelte-loader-hot
rixo 7783129
more affirmative
rixo e2d82d9
v0.1.1-0
rixo 8b31c10
v0.1.1
rixo b2d291c
bump svelte-hmr
rixo 472668f
v0.1.2
rixo 0c1ccaa
upgrade svelte-hmr (preservation of local state, reactive blocks, acc…
rixo 60b7ad9
v0.2.0
rixo ca2e6bd
update README
rixo ecaa242
v0.2.1
rixo 7367022
bump svelte-hmr (restore auto accept accessors & named exports -- fix…
rixo f148343
v0.3.0
rixo 0d7fb49
Bump acorn from 6.3.0 to 6.4.1
dependabot[bot] e2be8af
Merge pull request #2 from rixo/dependabot/npm_and_yarn/acorn-6.4.1
rixo 5d444ba
bump svelte-hmr to next
rixo 25e3244
v0.3.1-0
rixo df80f24
Bump lodash from 4.17.15 to 4.17.19
dependabot[bot] 4071bfc
Merge pull request #5 from rixo/dependabot/npm_and_yarn/lodash-4.17.19
rixo 1351638
upgrade to last svelte-hmr (fixes support for svelte >= 3.24.1)
rixo 42550eb
v0.3.1-1
rixo cd29428
v0.3.1
rixo e01f36a
Fix minor typo
rendall f88b4c6
Merge pull request #9 from rendall/patch-1
rixo 93feeb4
document that dev must be true for HMR
rixo 8b7d9bc
Merge branch 'master' of svelte-loader-hot into merge-rixo-hmr
non25 b5afe79
Drop Svelte 2 from HMR
non25 7e65739
Fix HMR test
non25 db9ff99
Move posixify back, remove unnecessary svelte-resolve
non25 780c961
Change backticks to single quotes
non25 931dcc9
Use matching imports in index.js
non25 1ff2a35
Merge README.md from rixo/svelte-loader#svelte-hmr
non25 ca550ea
Small changes that regards enabling emitCss and hotReload
non25 7925e76
Options: disable hotReload by default, remove shared
non25 0f1c49b
Revert hotReload
non25 bfb3b47
Options: remove externalDependencies
non25 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,51 +1,103 @@ | ||
import { Registry, configure as configureProxy, createProxy } from 'svelte-dev-helper'; | ||
import { makeApplyHmr } from 'svelte-hmr/runtime'; | ||
|
||
let hotOptions = { | ||
noPreserveState: false | ||
}; | ||
// eslint-disable-next-line no-undef | ||
const g = typeof window !== 'undefined' ? window : global; | ||
|
||
export function configure(options) { | ||
hotOptions = Object.assign(hotOptions, options); | ||
configureProxy(hotOptions); | ||
} | ||
const globalKey = | ||
typeof Symbol !== 'undefined' | ||
? Symbol('SVELTE_LOADER_HOT') | ||
: '__SVELTE_LOADER_HOT'; | ||
|
||
export function register(id, component) { | ||
if (!g[globalKey]) { | ||
// do updating refs counting to know when a full update has been applied | ||
let updatingCount = 0; | ||
|
||
//store original component in registry | ||
Registry.set(id, { | ||
rollback: null, | ||
component, | ||
instances: [] | ||
}); | ||
const notifyStart = () => { | ||
updatingCount++; | ||
}; | ||
|
||
//create the proxy itself | ||
const proxy = createProxy(id); | ||
const notifyError = reload => err => { | ||
const errString = (err && err.stack) || err; | ||
// eslint-disable-next-line no-console | ||
console.error( | ||
'[HMR] Failed to accept update (nollup compat mode)', | ||
errString | ||
); | ||
reload(); | ||
notifyEnd(); | ||
}; | ||
|
||
//patch the registry record with proxy constructor | ||
const record = Registry.get(id); | ||
record.proxy = proxy; | ||
Registry.set(id, record); | ||
const notifyEnd = () => { | ||
updatingCount--; | ||
if (updatingCount === 0) { | ||
// NOTE this message is important for timing in tests | ||
// eslint-disable-next-line no-console | ||
console.log('[HMR:Svelte] Up to date'); | ||
} | ||
}; | ||
|
||
return proxy; | ||
g[globalKey] = { | ||
hotStates: {}, | ||
notifyStart, | ||
notifyError, | ||
notifyEnd, | ||
}; | ||
} | ||
|
||
export function reload(id, component) { | ||
const runAcceptHandlers = acceptHandlers => { | ||
const queue = [...acceptHandlers]; | ||
const next = () => { | ||
const cur = queue.shift(); | ||
if (cur) { | ||
return cur(null).then(next); | ||
} else { | ||
return Promise.resolve(null); | ||
} | ||
}; | ||
return next(); | ||
}; | ||
|
||
const record = Registry.get(id); | ||
export const applyHmr = makeApplyHmr(args => { | ||
const { notifyStart, notifyError, notifyEnd } = g[globalKey]; | ||
const { m, reload } = args; | ||
|
||
//keep reference to previous version to enable rollback | ||
record.rollback = record.component; | ||
let acceptHandlers = (m.hot.data && m.hot.data.acceptHandlers) || []; | ||
let nextAcceptHandlers = []; | ||
|
||
//replace component in registry with newly loaded component | ||
record.component = component; | ||
m.hot.dispose(data => { | ||
data.acceptHandlers = nextAcceptHandlers; | ||
}); | ||
|
||
Registry.set(id, record); | ||
const dispose = (...args) => m.hot.dispose(...args); | ||
|
||
//re-render the proxy instances | ||
record.instances.slice().forEach(function(instance) { | ||
instance && instance._rerender(); | ||
const accept = handler => { | ||
if (nextAcceptHandlers.length === 0) { | ||
m.hot.accept(); | ||
} | ||
nextAcceptHandlers.push(handler); | ||
}; | ||
|
||
const check = status => { | ||
if (status === 'ready') { | ||
notifyStart(); | ||
} else if (status === 'idle') { | ||
runAcceptHandlers(acceptHandlers) | ||
.then(notifyEnd) | ||
.catch(notifyError(reload)); | ||
} | ||
}; | ||
|
||
m.hot.addStatusHandler(check); | ||
|
||
m.hot.dispose(() => { | ||
m.hot.removeStatusHandler(check); | ||
}); | ||
|
||
//return the original proxy constructor that was `register()`-ed | ||
return record.proxy; | ||
} | ||
const hot = { | ||
data: m.hot.data, | ||
dispose, | ||
accept, | ||
}; | ||
|
||
return Object.assign({}, args, { hot }); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
const { walk } = require('svelte/compiler'); | ||
const { createMakeHot } = require('svelte-hmr'); | ||
|
||
const hotApi = require.resolve('./hot-api.js'); | ||
|
||
const makeHot = createMakeHot({ | ||
walk, | ||
meta: 'module', | ||
hotApi, | ||
}); | ||
|
||
module.exports.makeHot = makeHot; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that @rixo's branch does not have the below options specified in
pluginOptions
I think
shared
does not exist and perhaps we don't want to enable the others by default?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did you get this from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
svelte-hmr
branch: https://github.com/rixo/svelte-loader/tree/svelte-hmrThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should only disable
hotReload
by default and removeshared
. The other two doesn't do anything intrue/false
manner.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the only thing it's used for is to determine if it's a compiler option:
svelte-loader/index.js
Line 81 in 0f1c49b
Since none of these are compiler options we should probably remove them all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to avoid making changes that are out of the scope of this PR.
Code could be improved, but in the other PR.
This place is hard to spot new comments on. 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, when I said "all" I meant the four options I originally suggested removing:
externalDependencies
,hotReload
,hotOptions
,shared
There shouldn't be any reason to pass those to the compiler since none of those four are valid compiler options: https://svelte.dev/docs#svelte_compile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this breaks tests and compilation. These options are PREVENTED from going to the compiler due to the
pluginOptions
object and code above.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. Sorry I had that backwards. It's a bit confusingly written. In
rollup-plugin-svelte
we moved them to be undercompilerOptions
. We should probably do that here too, but that can be a separate change. Just removingshared
and leaving the rest as-is would make senseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is done.
Yeah, that made me scratch my head for several minutes too. 😄