-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[🐞] qwik-city doesn't check if server$ calls are server$ #7313
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
Comments
do it in |
https://discord.com/channels/@me/1295311135586652191/1341655795703742495 wmertenssaid: it means having to read the q-manifest file at startup and finding symbols that look like this: "s_0EBpKMD7Km0": {
"origin": "../../qwik-router/lib/index.qwik.mjs",
"displayName": "index.qwik.mjs_serverQrl_rpc",
"canonicalFilename": "index.qwik.mjs_serverQrl_rpc_0EBpKMD7Km0",
"hash": "0EBpKMD7Km0",
"ctxKind": "function",
"ctxName": "$",
"captures": true,
"parent": null,
"loc": [
53568,
56525
]
}, So the displayName ending with serverQrl_rpc and ctxKind function. And then in the place where it executes qFunc, check the list this is context |
By looking at the symbol name you can see if it's a server qrl, so no optimizer changes needed. However, it would be great if the optimizer could list out the function argument names and captured scope names. Then we can generate an API report that should be checked in, which helps show when the API changes. |
@JerryWu1234 I'm not sure I understand the issue (can't access your link), but I think you can do a first PR without the change for the optimizer; and create a separate issue for the optimizer optimization. |
Which component is affected?
Qwik City (routing)
Describe the bug
qwik-city should only allow calling server$ and other qrls if they are generated as server$.
Right now any valid qrl can be called. This is somewhat academic but still a concern.
The text was updated successfully, but these errors were encountered: