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.

Sign up for GitHub

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

Jump to bottom

Ability to set inner instance #1

Merged
merged 8 commits into from
Dec 3, 2021
Merged

Ability to set inner instance #1

merged 8 commits into from
Dec 3, 2021

Conversation

duart38
Copy link
Collaborator

@duart38 duart38 commented Nov 1, 2021

As it stand you can't adjust the inner state object directly by assignment (after getInstance). there's no reason this shouldn't be possible.

singleton.getInstance() = {..}; // error
singleton.setInstance(...); // works

@duart38 duart38 requested a review from grevend November 1, 2021 20:01
@duart38 duart38 self-assigned this Nov 1, 2021
@grevend grevend added enhancement New feature or request good first issue Good for newcomers labels Nov 1, 2021
@grevend grevend changed the title ability to set inner instance Ability to set inner instance Nov 1, 2021
grevend
grevend requested changes Nov 1, 2021
View reviewed changes
Copy link
Owner

@grevend grevend left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. Before I can merge your pull request, it would be important to add the necessary test cases and update the documentation.

@duart38 duart38 requested a review from grevend December 3, 2021 13:00
grevend
grevend requested changes Dec 3, 2021
View reviewed changes
test.ts Outdated Show resolved Hide resolved
@duart38 duart38 requested a review from grevend December 3, 2021 13:21
grevend
grevend requested changes Dec 3, 2021
View reviewed changes
Copy link
Owner

@grevend grevend left a comment

Choose a reason for hiding this comment

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

Please also update the documentation (README).

singleton.bundle.js Show resolved Hide resolved
@duart38 duart38 requested a review from grevend December 3, 2021 13:30
@grevend grevend merged commit b54bf26 into main Dec 3, 2021
@grevend grevend deleted the container-setter branch December 3, 2021 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grevend grevend Awaiting requested review from grevend

Assignees

@duart38 duart38

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@duart38 @grevend