-
-
Notifications
You must be signed in to change notification settings - Fork 197
Kddimitrov/restart on hmr fail #3980
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
Conversation
99642ab
to
2e7bc0a
Compare
@@ -0,0 +1,4 @@ | |||
interface IHmrStatusService { | |||
awaitHmrStatus(deviceId: string, operationHash: string): Promise<number>; |
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.
awaitHmrStatus -> getHmrStatus
lib/services/hmr-status-service.ts
Outdated
const interval = setInterval(() => { | ||
const status = this.getStatusByKey(key); | ||
if (status || retryCount === 0) { | ||
clearInterval(interval); |
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.
+ clear on dispose
|
||
if (data.appFilesUpdaterOptions.useHotModuleReload && currentHmrData.hash) { | ||
const devices = _.filter(this.$previewSdkService.connectedDevices, { platform: platform.toLowerCase() }); | ||
_.forEach(devices, async (previewDevice: Device) => { |
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.
await properly here
lib/services/log-parser-service.ts
Outdated
} | ||
|
||
private processDeviceLogResponse(message: string, deviceIdentifier: string) { | ||
const device = this.$devicesService.getDeviceByIdentifier(deviceIdentifier); | ||
const devicePlatform = device.deviceInfo.platform.toLowerCase(); | ||
const devicePlatform = this.tryGetPlatform(deviceIdentifier); |
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.
check the performance implications here (we will get the device or catch an exception for each log)
const status = await this.$hmrStatusService.awaitHmrStatus(previewDevice.id, currentHmrData.hash); | ||
if (status === HmrConstants.HMR_ERROR_STATUS) { | ||
await this.applyChanges(platformData, projectData, currentHmrData.fallbackFiles[platform], false); | ||
} |
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.
boolean variable passed as argument 🙀 🙀 🙀
await this.applyChanges(platformData, projectData, currentHmrData.fallbackFiles[platform], { useHotModuleReload: false });
a8f42a5
to
0c5b380
Compare
0c5b380
to
4e94cb1
Compare
PR Checklist
What is the current behavior?
When HMR fails to apply patch the application will remain with old content.
What is the new behavior?
When HMR fails to apply patch the application gets updated by regular LS and restarted.
Fixes/Implements/Closes #3892.