Skip to content

fix(webpack): Method View._onLiveSync context is always undefined #9839

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

fix(webpack): Method View._onLiveSync context is always undefined #9839

wants to merge 4 commits into from

Conversation

CatchABus
Copy link
Contributor

@CatchABus CatchABus commented Mar 22, 2022 •
edited
Loading

PR Checklist

What is the current behavior?

Method View._onLiveSync argument is always undefined.

What is the new behavior?

Method View._onLiveSync argument is an object that implements ModuleContext interface as expected.

There is the need of context in a few places inside core modules. Few examples:

NativeScript/packages/core/ui/core/view/view-common.ts

Line 220 in 6941466

public _handleLivesync(context?: ModuleContext): boolean {

NativeScript/packages/core/ui/frame/frame-common.ts

Line 625 in f70081d

public _handleLivesync(context?: ModuleContext): boolean {

NativeScript/packages/core/application/index.ios.ts

Line 326 in 650ef59

public _onLivesync(context?: ModuleContext): void {

Fixes/Implements/Closes #9837 .

This is my proposal for solving the issue related.
I took the liberty and replaced the old hot-loader approach of reading a file source and inject it in runtime, with a new function that contains code and the use of toString() call. This prints the source code of the function.

UPDATED:
When hmr checks and retrieves modules that will apply changes to, a context argument will be set on __onLiveSync call.

There is also this case which runs if injectHMRRuntime is set to false: https://github.com/DimitrisRK/NativeScript/blob/8855ca43735f6e6f6ffd3598ff3ea2d18d57fbbd/packages/webpack5/src/loaders/nativescript-hot-loader/index.ts#L30
I wasn't sure if I should meddle with this code injection, so I left it intact.

@rigor789 Provided that we have found this patch okay or have applied all necessary changes, there might be a need for few changes in core modules too. Would it be alright to add those changes in the same PR?

@cla-bot cla-bot bot added the cla: yes label Mar 22, 2022
@rigor789 rigor789 self-assigned this Mar 24, 2022
@CatchABus
Copy link
Contributor Author

As mentioned in #6665, this mechanism was implemented for the following reasons:

Expose HmrContext interface.
Apply changes in app.css or app.scss instantly.
Avoid navigation on livesync when changes in app.css or app.scss have been made.
Apply changes in app.css or app.scss on back navigation.

Core contains dozens of ugly checks for such minor details.
Perhaps, this doesn't need much attention for now as my solution will also need several tweaks inside core to work.
On the other hand, the page history problem I encountered during live-sync (which is the reason I reported this) can be easily solved so we may focus on this one.

@CatchABus CatchABus mentioned this pull request Apr 30, 2022
Merged
5 tasks
@CatchABus CatchABus marked this pull request as draft November 11, 2022 10:38
@CatchABus
Copy link
Contributor Author

I close this as it's a vanilla-specific feature that worked with webpack 4. We need features that all flavors can benefit from.

@CatchABus CatchABus closed this Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@rigor789 rigor789

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug(webpack5): Method View._onLiveSync context is always undefined
2 participants
@CatchABus @rigor789