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

HMR/live-reload fixes (webpack-dev-server) #7104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ixrock
Copy link
Member

@ixrock ixrock commented Feb 3, 2023

As part of #6948 since incorrectly rebased from first monorepo times.

Current master: only manual page refresh, e.gCmd+R (example of editing welcome.tsx)

Screen.Recording.2023-02-03.at.14.31.57.mov

PR changes:

Does some auto page-reloading, but doesn't respect cluster frame location, therefore leads to updating page to global catalog page which is not desired, but better then nothing. (needs to be improved in follow up PRs)

Screen.Recording.2023-02-03.at.14.24.36.mov

Current error leading to full page reload instead of HMR (caught from video above):

Screenshot 2023-02-03 at 14 41 29

…tion while reloading, part of #6948

Signed-off-by: Roman <ixrock@gmail.com>
@ixrock ixrock added the bug Something isn't working label Feb 3, 2023
@ixrock ixrock added this to the 6.10.0 milestone Feb 3, 2023
@ixrock ixrock requested a review from a team as a code owner February 3, 2023 12:48
@ixrock ixrock self-assigned this Feb 3, 2023
Comment on lines +38 to +40
// proxy: {
// "^/$": "/build/",
// },
Copy link
Member Author

Choose a reason for hiding this comment

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

@jakolehm why do we need this proxy at all? any ideas? it seem like works fine without this..

Copy link
Contributor

@aleksfront aleksfront left a comment

Choose a reason for hiding this comment

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

Not very useful with stated behavior: always reloading the page and moving to Catalog. Much easier to hit Cmd+R while in Cluster view because moving back from Catalog requires "bigger effor".

Also, HMR works as expected now when changing CSS.

@ixrock
Copy link
Member Author

ixrock commented Feb 4, 2023

Not very useful with stated behavior: always reloading the page and moving to Catalog. Much easier to hit Cmd+R while in Cluster view because moving back from Catalog requires "bigger effor".

@aleksfront Actually I think it should do page reloading in case when HMR cannot be applied and it works fine for app's global window (e.g. it keeps latest page, /welcome, /settings) but doesn't work inside cluster iframe properly for some reasons..
And that thing should be fixed or at least ignored (handling HMR inside iframe differently), so you could do manual page reloading whenever you want.

Any ideas why this error happens leading to full page reload?

image

@Nokel81 Nokel81 modified the milestones: 6.10.0, 6.5.0 Feb 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Nokel81 Nokel81 modified the milestones: 6.5.0, 6.6.0 May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR: needs rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants