feat: implement getServerFunctionMeta

[katywings]

This adds a new public getServerFunctionMeta API that can be used in "use server" functions to retrieve function related information. It can be used by the enduser as a stable key for server function related caches and debugging.

Tasks

• Implement basic functionality
• Figure out why importing getRequestEvent in @solidjs/start/server breaks the vite dev server
• Discuss and eventually change the naming of getRpcInfo / RpcInfo
  • Renamed to getServerFunctionMeta / ServerFunctionMeta
• Write docs
• Write tests - (if there is a place to write tests ;))

PR Checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• infrastructure changes
• Other... Please describe:

What is the current behavior?

Related discussion: nksaraf/vinxi#51

[nksaraf]

since this is related to server functions, lets use some terminology that matches that. event.serverFunction maybe? And the helper could be getServerFunctionEvent which is a wrapper around getRequestEvent.

But I am still not sure we should expose name and id separately. These are implementation details. If its a stable key that's needed. I think we should only have id maybe which should be opaque. No guarantees about it being same in dev and prod, etc. Just that it'll be stable for one version of the code in that build.

[katywings]

@nksaraf Would you construct the id how I did it earlier? (id + "#" + name)

Do you think this serverFunctionEvent object could include other stuff in the future? If we only return the id, how about we just call the helper getServerFunctionId?

Alternatively if you have ideas/plans to include other stuff, how about getServerFunctionMeta? I think Meta better communicates that the information does not change on each request. Event sounds like a thing that changes over time.

[nksaraf]

Alternatively if you have ideas/plans to include other stuff, how about getServerFunctionMeta? I think Meta better communicates that the information does not change on each request. Event sounds like a thing that changes over time.

Yup might be better to go with Meta (and just expose the id in there). The id using id#name is perfect I think. In production usually we are going to has the id part anyway, so its fine.

[katywings]

@nksaraf I renamed it to getServerFunctionMeta and changed the id. I also just tested again the issue with the getRequestEvent with vinxi v0.0.64 . Importing getRequestEvent in the start/server entry still crashes the dev server with the following error:

node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "true".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

Do you have an idea why this error is happening and how it can be prevented? I saw that start/server/spa/StartServer.tsx also imports and uses getRequestEvent without an error. Why does it work there, but fails in my start/server/serverFunction.tsx file?

[katywings]

@nksaraf Any ideas why the getRequestEvent import in solid-start breaks the dev server 😅? This is the only reason why this PR is hanging in "WIP" 🙈

[katywings]

Retested this with all latest versions. The dev server crash from importing getRequestEvent in solid-start internals has been resolved. My random guess is that the error might have been related to c586f14.

Anyhow, this PR is ready for further discussion now :).

[ryansolid]

My only thought here is event.context is an H3 thing not a start thing so I'd consider using a different location. I haven't really standardized that though.. there is event.locals.

I guess we need to make a decision here soon. Right now it is my undocumented playground, but we need to do better.

[katywings]

event.context is an H3 thing ... there is event.locals. ...

Okay that is weird, Vscode is showing me h3 types for getRequestEvent in my local solid-start workspace, so I assumed that internally in solid-start getRequestEvent directly returns and uses the h3 event.

Edit:

Wait, so RequestEvent is an extension of h3 event and mixes solid-start and h3? Afaik h3 specifically provides context as a place for frameworks and libraries to store their data - why would solid start have its own extra place? Nuxt seems to also use event.context like here for example: https://github.com/nuxt/nuxt/blob/406b204640bbed59703eaeef55eae8e3c0f37625/packages/nuxt/src/core/runtime/nitro/no-ssr.ts#L5

The event context of h3 could theoretically be extended with solid-start stuff like so:

declare module 'h3' {
  interface H3EventContext {
    start: {
      ...all of the start magic...
    }
  }
}

But maybe there is also a discussion with @pi0 worth to have about this?

[pi0]

Yes, that's precisely the purpose of event.context to be extended (and typed) by the frameworks which I wish solid adopted in first place instead of making more wrappers...

[katywings]

@ryansolid How do we move forward with this PR? 😅

[changeset-bot]

⚠️ No Changeset found

Latest commit: 36b7912

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ryansolid]

Yes, that's precisely the purpose of event.context to be extended (and typed) by the frameworks which I wish solid adopted in first place instead of making more wrappers...

Unfortunately we had this convention before we adopted H3/Nitro. And there is consideration here that goes beyond SolidStart.

[katywings]

Gotcha, I am looking forward what you will come up with 🙏. Just regarding "conventions": the new solid start beta changed a lot of things, removed features, removed whole apis, added others, some users have to migrate a lot of code (such as myself), I dont grasp why suddenly with the topic of event locals we should not rethink old conventions. I mean, who if not you, the CEO of signals, is not constantly reworking, rethinking, evolving old conventions 😉. I get that this goes beyond SolidStart 👍.

[katywings]

Fixed merge conflicts and updated the feature to the current event structure.

[katywings]

@ryansolid Any news on this? 😅

[ryansolid]

It was easier to cherry pick the commit than merge the merge after this took so long to get in. I will close this. But it is getting in.

[katywings]

It was easier to cherry pick

Yeah that makes absolutely sense 😅👍.

But it is getting in.

Thats some awesome news, thank you!