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(fe2): SSR memory leak caused by vue apollo #3790

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

Conversation

fabis94
Copy link
Contributor

@fabis94 fabis94 commented Jan 9, 2025

you can find the full source code of the fork as well as my specific changes through this PR: https://github.com/vuejs/apollo/pull/1582/files

@@ -45,6 +45,7 @@
}
},
"peerDependencies": {
"vee-validate": "^4.7.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

made it a peerDep so that it can be independently controlled

@@ -77,7 +77,7 @@
"tweetnacl-sealedbox-js": "^1.2.0",
"tweetnacl-util": "^0.15.1",
"ua-parser-js": "^1.0.38",
"vee-validate": "^4.7.0",
"vee-validate": "4.7.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's been quite a few changes and I do want to upgrade to the latest version, but there's certain breaking changes so lets do it separately

@@ -59,6 +59,7 @@
"husky": "^7.0.4",
"lint-staged": "^12.3.7",
"lockfile": "^1.0.4",
"npkill": "^0.12.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for explicitly adding it as a dev dependency?
I npx npkill normally. But I don't see a script in package.json using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i dislike using npx in a yarn project, because it uses an entirely different package manager to install the dep in a yarn project. you don't need a script in package.json, you can just do yarn npkill

@@ -53,7 +53,7 @@
"@tiptap/suggestion": "2.10.3",
"@tiptap/vue-3": "2.10.3",
"@tryghost/content-api": "^1.11.21",
"@vue/apollo-composable": "4.0.2",
"@vue/apollo-composable": "npm:@speckle/[email protected]",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you point to the source code of the fork please @fabis94 ?
(and add the link to the description of this PR please, so we can trace provenance)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iainsproat i added a link to the description

Mikehrn
Mikehrn previously approved these changes Jan 9, 2025
package.json Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jan 9, 2025

📸 Preview service has generated an image.

iainsproat
iainsproat previously approved these changes Jan 9, 2025
@fabis94 fabis94 dismissed stale reviews from iainsproat and Mikehrn via 873d54c January 10, 2025 11:26
Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

@fabis94 fabis94 requested review from Mikehrn and iainsproat January 10, 2025 11:56
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.

3 participants