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

Fix #9960 by adding hot-code-push to minimal skelet #11412

Merged
merged 7 commits into from
Jun 12, 2021

Conversation

StorytellerCZ
Copy link
Collaborator

Adding hot-code-push to minimal skeleton fixes and issue where the server would reload when a change occurred on client.

@StorytellerCZ
Copy link
Collaborator Author

@filipenevola @renanccastro what are your thoughts on this change?

@filipenevola
Copy link
Collaborator

I believe the root cause is the same problem that I'm fixing here #11381

This PR is going to be closed as @zodern is going to implement a different approach to solve the same issue.

@zodern do you agree with me? Because if your fix is going to fix this restart for the minimal as well, so we don't need to merge this PR.

@filipenevola filipenevola added the in-discussion We are still discussing how to solve or implement it label May 12, 2021
zodern
zodern previously approved these changes May 13, 2021
@zodern zodern dismissed their stale review May 13, 2021 20:32

misunderstood the change

@zodern
Copy link
Member

zodern commented May 13, 2021

Is the reason hot-code-push is missing because it increased the production bundle size?

If that is still a concern, one option might be to add Autoupdate as a dependency to hot-module-replacement (I'm not sure why it isn't already), and add hot-module-replacement instead. Since it is a debug only package, it and Autoupdate should be excluded from the production bundle. This might also solve meteor/meteor-feature-requests#354.

@zodern
Copy link
Member

zodern commented May 13, 2021

@zodern do you agree with me? Because if your fix is going to fix this restart for the minimal as well, so we don't need to merge this PR.

Meteor always does a full rebuild unless the app uses the autoupdate package:

// We only can refresh the client without restarting the server if the
// client contains the 'autoupdate' package.
var canRefreshClient = self.projectContext.packageMap &&
self.projectContext.packageMap.getInfo('autoupdate');

The changes I am making to watching won't change that.

@StorytellerCZ StorytellerCZ removed this from the Release 2.3 milestone May 17, 2021
@StorytellerCZ
Copy link
Collaborator Author

Changing for autoupdate package.

@StorytellerCZ StorytellerCZ requested a review from zodern May 20, 2021 18:07
@StorytellerCZ StorytellerCZ added this to the Release 2.3 milestone May 20, 2021
@StorytellerCZ
Copy link
Collaborator Author

autoupdate or hot-code-push either fix the issue. HCP refreshes the browser, autoupdate does not. The issue remains of the bundle size.

@StorytellerCZ StorytellerCZ added in-development We are already working on it and removed in-discussion We are still discussing how to solve or implement it labels May 27, 2021
@StorytellerCZ StorytellerCZ removed this from the Release 2.3 milestone Jun 4, 2021
@filipenevola
Copy link
Collaborator

@StorytellerCZ let's resume what is going on here.

Did you try to add hot-code-push to minimal but it added more code to the bundle size, is that the issue with this approach? How much was added?

I would like to hear from people using the minimal in production. Is it a real use case? I will ask about this in our community Slack.

@StorytellerCZ StorytellerCZ removed the in-development We are already working on it label Jun 11, 2021
@StorytellerCZ StorytellerCZ self-assigned this Jun 11, 2021
@StorytellerCZ StorytellerCZ added this to the Release 2.3 milestone Jun 11, 2021
@StorytellerCZ StorytellerCZ changed the base branch from devel to release-2.3 June 11, 2021 20:07
@StorytellerCZ StorytellerCZ merged commit 025af3e into release-2.3 Jun 12, 2021
@StorytellerCZ StorytellerCZ deleted the hot-code-push-skelet-minimal branch June 12, 2021 10:21
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.

Meteor create --minimal is forcing meteor server restart on client changes
3 participants