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

Add more specific parameter type for setData() method #114

Merged
merged 1 commit into from
Aug 22, 2022
Merged

Conversation

maximal
Copy link
Contributor

@maximal maximal commented May 11, 2022

FCM message will not be received if array shape is different from array<string, string> (string keys and string values).

For example, this code won’t work:

return FcmMessage::create()
	->setData([
		'string_key1' => 'string_value1',
		'string_key2' => 'string_value1',
		'string_key3' => 666, // integer value
	])->...

I propose to specify an array shape for the parameter of setData() method in order to help IDEs to analyse the code and to highlight possible errors preemptively. This will save developers’ time (I personally spent on this issue a couple of investigation hours).

FCM message will not be received if array shape is different from array<string, string> (string keys and string values).

For example, this code won’t work:
```php
return FcmMessage::create()
	->setData([
		'string_key1' => 'string_value1',
		'string_key2' => 'string_value1',
		'string_key3' => 666, // integer value
	])->...
```

I propose to specify array shape for the parameter of `setData()` method in order to help IDEs to analyse the code and to highlight possible errors preemptively. This will save developers’ time (I personally spent on this issue a couple of investigation hours).
@atymic atymic merged commit 5784e76 into laravel-notification-channels:master Aug 22, 2022
@maximal maximal deleted the patch-1 branch January 11, 2023 12:32
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.

2 participants