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: reduce dependency on the Client class #122

Closed
wants to merge 10 commits into from

Conversation

TTtie
Copy link
Member

@TTtie TTtie commented Oct 19, 2023

This PR introduces changes that aim to reduce the dependency of data structures on the Client class (and internal cached state in general) in order to prepare for the REST and WS parts of the client being split in #52.

@TTtie TTtie added type: enhancement New feature or request scope: meta Meta changes scope: structures Issues or pull requests affecting the structure definitions labels Oct 19, 2023
@TTtie TTtie added this to the 0.2.0 milestone Oct 19, 2023
@TTtie TTtie self-assigned this Oct 19, 2023
@TTtie TTtie force-pushed the refactor/cachelessness branch from 22f1ea3 to 2cdadca Compare November 28, 2023 11:12
@lnpotter
Copy link
Contributor

Does this mean that you intend to deflate the Client structure by moving some methods and functions?

@TTtie
Copy link
Member Author

TTtie commented Dec 16, 2023

Not really. The intended end goal for this PR is to make the data structures account for a possibility of not having internal state given to them and act appropriately if it isn't.

In an ideal case, this should just work (T being an appropriate data structure):

import { T } from "@projectdysnomia/dysnomia";

const rawStruct: RawAPIData<T> = { /* ... */ };

// apiObject is now a fancy wrapper around rawStruct
// It must not error if a Client isn't passed as a second parameter
const apiObject = new T(rawStruct);

// Properties should just work, as they are stored directly on the API object
console.log(apiObject.id);
console.log(apiObject.aThing);

// Calling methods on such an object, however, can end up 
// in an error if a Client is needed to do the operation.
// An error like this will be thrown synchronously in most cases.
// (might possibly be subject to change)
try {
    await apiObject.doSomething();
} catch (err: unknown) {
    if (err instanceof TypeError) {
        console.error("A Client was not available on this apiObject");
    } else {
        console.error("A regular error occurred");
    }
    console.error(err);
}

@TTtie TTtie added the help wanted Extra attention is needed label Apr 10, 2024
@TTtie TTtie modified the milestones: 0.2.0, next Jul 13, 2024
@TTtie
Copy link
Member Author

TTtie commented Aug 16, 2024

this is definitely still on the table, but fwiw, this might be better to do as multiple separate PRs for each thing and probably with slightly better research on this.

@TTtie TTtie closed this Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed scope: meta Meta changes scope: structures Issues or pull requests affecting the structure definitions type: enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants