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

feat(ssr): add ability to cleanup after request #9479

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

ronald-d-rogers
Copy link

@ronald-d-rogers ronald-d-rogers commented Feb 12, 2019

close #9463

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
The point of this "feature" is to be able to do something to the app to mark it as "invalid" so that vue-router has a way to detect and cleanup objects it might have left hanging around in memory. This feature might be able to be used for other reasons, but it is probably truly a fix. Also, a change would need to be approved in vue-router as well for the memory leak there to even go away (see vuejs/vue-router#2606), and that might get fixed by other means, so this might not be worth doing anything with if that doesn't also get approved.

Because we're in control of when the app is created in SSR it didn't seem like there was any other sensible option but to give us control of when to clean it up, though maybe something can be done based on runInNewContext. Also, maybe a more explicit API would be better, e.g. some way to mark the app as "served", $isServer && $isServed.

Note I'm submitting this knowing that there is a good chance that this will be tossed, so feel free to do so if this is not what is wanted.

@SacDin
Copy link

SacDin commented Jun 15, 2019

Is it going to be merged ?

@ronald-d-rogers
Copy link
Author

PR made in vue-router illustrating what would need to be done there as well: vuejs/vue-router#2867

Again I am skeptical as to whether or not this is the right approach, but I am skeptical of everything I do so take that for what it's worth.

@ronald-d-rogers
Copy link
Author

Note I made another PR made in vue-router illustrating a way to fix the leak without requiring this PR: vuejs/vue-router#2875

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide way to destroy app in SSR
3 participants