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

make decorators readonly when accessed through context #176

Draft
wants to merge 1 commit into
base: miyu
Choose a base branch
from

Conversation

ethanniser
Copy link

@ethanniser ethanniser commented Sep 18, 2023

The change is like 2 lines, but seems to have had a ripple effect causing a couple type errors, which I am working to fix

But I would still like your thoughts in the meantime


This pr:

  • changes the type of context to make decorators readonly
  • adds a test

Why?

  • currently it is a bit confusing what is the difference between state and decorate, to a user it feels like they are the same thing just one ends up in ctx and one in ctx.store
  • I would argue they should have a semantic difference, where decorators are immutable (ie a database client, or a config object) and shouldn't be modified, where state is mutable and can be modified
    There is this part from the context page of the docs which I believe shares this sentiment:

store: A global mutable store for Elysia instance

Making decorators readonly makes it more clear when a user should use state vs decorate, and provides additional safety for large, important global values you want ensure aren't modified

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.

None yet

1 participant