Skip to content

Conversation

@cherryblossom000
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:

Use readonly arrays in parameters. For example, these now work:

declare const guild: Guild
declare const positions: readonly ChannelPosition[]
guild.setChannelPositions(positions)

declare const member: GuildMember
declare const permissions: readonly PermissionString[]
member.permissions.has(permissions)

T[] is assignable to readonly T[], so the following will continue to work:

declare const positions2: ChannelPosition[]
guild.setChannelPositions(positions2)

declare const permissions2: PermissionString[]
member.permissions.has(permissions2)

Status

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

cherryblossom000 and others added 2 commits August 12, 2020 09:44
Use readonly arrays in parameters. For example, these now work:

```ts
declare const member: GuildMember
declare const permissions: readonly PermissionString[]
member.permissions.has(permissions)

declare const guild: Guild
declare const positions: readonly ChannelPosition[]
guild.setChannelPositions(positions)
```
@iCrawl iCrawl changed the title types: use readonly arrays in parameters feat(typings): use readonly arrays in parameters Aug 13, 2020
@iCrawl iCrawl merged commit f451be0 into discordjs:master Aug 13, 2020
@cherryblossom000 cherryblossom000 deleted the typings branch August 13, 2020 21:50
@wasdennnoch
Copy link
Contributor

This change breaks code where you're doing something like this:

const embed: MessageEmbedOptions = {
	fields: [{
		// fixed stuff
	}],
};
if (/* condition */) {
	embed.fields.push({
		// conditional stuff
	});
}

Because embed.fields is now marked as readonly you can't push into it anymore.

@cherryblossom000
Copy link
Contributor Author

@wasdennnoch Hmm, I didn't think about that. I think this is a more common use case than using readonly arrays, so I've created #4794.

iCrawl pushed a commit that referenced this pull request Sep 5, 2020
This reverts some of the changes in f451be0 so that this works:

```ts
const embed: MessageEmbedOptions = {
  fields: [{
    // fixed stuff
  }],
};
if (/* condition */) {
  embed.fields.push({
    // conditional stuff
  });
}
```

See #4692 (comment).
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.

3 participants