Skip to content
This repository has been archived by the owner on Jan 8, 2022. It is now read-only.

feat: add Collection#ensure #52

Merged
merged 26 commits into from Dec 24, 2021
Merged

feat: add Collection#ensure #52

merged 26 commits into from Dec 24, 2021

Conversation

MidSpike
Copy link
Contributor

This PR is a continuation of #40 which was closed by the author.

Inspired by Enmap#ensure, Collection#ensure gets an element with the specified key if it exists, otherwise sets it to the provided defaultValue and returns the defaultValue. Useful for things like per-guild settings where you want to get it or set it to a default value all in one call.

I found @kxmndz 's PR to be useful and wanted to ensure that it was maintained.

Potential Usage Example:

const guildConfigs = new Discord.Collection();
const defaultGuildConfig = {
  allowInvitesInChat: true,
};
const guildConfig = guildConfigs.ensure('566340180831633429', () => defaultGuildConfig);

Noticeable deviations from PR #40:

  • The PR builds off of feat: add Collection#ensure #40 (rebased)
  • The expected return value from collection#ensure can now be undefined|null whereas before a non-nullish return value was expected.
  • 2 parameters have been added to the generator function key: K, collection: this.
    I believe that recycling these into the generator function will allow for less repetition in the end-user's code.
  • @kyranet 's suggested edit was considered but directly interferes with the ability to have undefined|null as a valid ensured value, therefore it was not implemented into this PR.

Tests:
I'm not good at writing tests and would appreciate if someone help could contribute tests to cover the additional functionality added by this PR compared to the tests carried over by #40.

Status and versioning classification:

  • Code changes have been tested, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)

kxmndz and others added 14 commits November 24, 2021 12:01
Inspired by `[Enmap](https://enmap.evie.dev)#ensure`, `Collection#ensure` gets an element with the specified key if it exists, otherwise sets it to the provided `defaultValue` and returns the `defaultValue`. Useful for things like per-guild settings where you want to either get it or set it to a default value.
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
Signed-off-by: Kyran Mendoza <mendozakyran8@gmail.com>
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
__tests__/collection.test.ts Outdated Show resolved Hide resolved
__tests__/collection.test.ts Outdated Show resolved Hide resolved
__tests__/collection.test.ts Outdated Show resolved Hide resolved
__tests__/collection.test.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
MidSpike and others added 3 commits November 24, 2021 14:14
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

Just one last thing, let's keep the setup step tidy :P

__tests__/collection.test.ts Outdated Show resolved Hide resolved
__tests__/collection.test.ts Outdated Show resolved Hide resolved
__tests__/collection.test.ts Outdated Show resolved Hide resolved
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

Now for real, one last thing before LGTM, sorry!

src/index.ts Outdated Show resolved Hide resolved
MidSpike and others added 2 commits November 24, 2021 14:41
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
kyranet
kyranet previously approved these changes Nov 24, 2021
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
implemented suggested changes by kyranet

Co-authored-by: muchnameless <12682826+muchnameless@users.noreply.github.com>
src/index.ts Outdated Show resolved Hide resolved
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
__tests__/collection.test.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
MidSpike and others added 3 commits November 24, 2021 19:17
Co-authored-by: Rodry <38259440+ImRodry@users.noreply.github.com>
Co-authored-by: Rodry <38259440+ImRodry@users.noreply.github.com>
Co-authored-by: Rodry <38259440+ImRodry@users.noreply.github.com>
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

One little documentation thing, then I'll re-approve.

src/index.ts Outdated Show resolved Hide resolved
implemented suggested change

Co-authored-by: Antonio Román <kyradiscord@gmail.com>
@ImRodry
Copy link
Contributor

ImRodry commented Dec 23, 2021

Any ETA for when this will be merged? Would love to see this and #48 in a future release!

@iCrawl iCrawl merged commit 3809eb4 into discordjs:main Dec 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants