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

Refactor action code strings into an enum-like object #570

Merged
merged 6 commits into from
Aug 9, 2022

Conversation

ericyhwang
Copy link
Contributor

Action codes used in request.a are currently string literals in the source code, e.g. qf (query fetch) or s (subscribe). That makes the code harder to navigate and learn.

This PR refactors them into an enum-like object, such that they can be used like Action.QueryFetch.

As a bonus, it'll help if/when we find the time to switch to TypeScript, too!

Things to discuss and look at:

  1. Location of the object. I created a new shared/ directory for it, since it's code shared between client and server.
  2. Naming and casing of the object and its keys. For casing, I decided to go with the UpperCamelCase used by TS-style enums, but I'm open to arguments for other casing or names.
  3. Completeness and correctness. I did a search for the strings and and looked at each non-test usage one-by-one, aiming to only update the strings that were actually action codes. One example of something I didn't change was the calls to Connection#sendBulk, where the strings aren't action codes directly, since they're later prefixed with "b":
    this._sendBulk('f', collection, actions.f);
    this._sendBulk('s', collection, actions.s);
    this._sendBulk('u', collection, actions.u);
  4. Speaking of tests, I decided to not update the action code string literals in tests, so the tests can continue to serve as a guard against someone accidentally changing the action codes.

@coveralls
Copy link

coveralls commented Aug 9, 2022

Coverage Status

Coverage increased (+0.006%) to 97.416% when pulling bbeaeb9 on refactor-action-codes into 2532faa on master.

@ericyhwang
Copy link
Contributor Author

  • Move from shared/ to top level, since other shared things are at the top level.
  • Switch to ACTIONS.queryFetch casing

@ericyhwang ericyhwang merged commit 28460e3 into master Aug 9, 2022
@ericyhwang ericyhwang deleted the refactor-action-codes branch August 9, 2022 17:19
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

3 participants