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: 🐛 allow typed server methods access cache methods #4515

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

damusix
Copy link
Contributor

@damusix damusix commented Jul 2, 2024

also fix pre typing issue would be fixed by #4489 . Change removed.

@kanongil
Copy link
Contributor

kanongil commented Jul 2, 2024

Fix a pre typing issue? Can you elaborate? To me it reads like you created an issue with this PR.

@damusix
Copy link
Contributor Author

damusix commented Jul 2, 2024

@kanongil I like the strong typing, but for practical purposes, you can't use utilities that don't carry the same Refs.

If you run it locally, observe the type test:


        pre: [
            {
                assign: 'some',
                method: ((req) => {

                    return req.app.word;
                }) as Lifecycle.Method<RequestDecorations>
            },
            {
                assign: 'another',
                method: getNum
            }
        ]

In the current state of typings, where one is used as a utility (getNum) and the other is defined in the object. When we define pre[].method() to take a Refs generic, it wants all pres to take the same Refs generic, or else it errors.

If you have other ideas, that'd be great, cause I agree that this is sub-optimal.

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.

2 participants