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

Add injected isomorphic environment #4

Merged
merged 9 commits into from
Sep 27, 2023
Merged

Conversation

Monkatraz
Copy link
Contributor

This makes it so that when you create a server, you need to provide an environment to the server. The server will then pass this environment to all procedures that are called. Environments are always the same shape.

@Monkatraz
Copy link
Contributor Author

oh hold on there are tests I need to change

router/server.ts Outdated
Comment on lines 34 to 38
const context: ProcedureContext<object> = {
environment,
state: service.state,
};

Copy link
Member

Choose a reason for hiding this comment

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

isn't this a ServiceContext? would also be good to have a mapping of service -> ServiceContext so that we avoid constructing it on each request

Comment on lines 2 to 3
log: (...args: unknown[]) => void;
error: (...args: unknown[]) => void;
Copy link
Member

Choose a reason for hiding this comment

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

can we add a comment here about declaration merging and then add a test that adds something to the environment?

@Monkatraz
Copy link
Contributor Author

not sure how to do the declaration merging test without affecting the exported type

@Monkatraz
Copy link
Contributor Author

okay, this should be good to go

@jackyzha0
Copy link
Member

comments addressed after discussion in-person the day before :)

@Monkatraz Monkatraz merged commit 107db3d into main Sep 27, 2023
4 checks passed
@jackyzha0 jackyzha0 deleted the brn-isomorphic-environment branch November 16, 2023 23:34
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