fix: Incorrect type for SQS messageGroupId (#1205)

The current type is `null | undefined`, which is unusable!

I've changed this to `string | null | undefined` as a non-breaking step
torwards `string | undefined`, with plans to remove the `null` type in
v3.

Also improves associated JSDoc.

Author: seb-cr

docs/migration/v2.md

@@ -172,9 +172,11 @@ In v1, the default behaviour of `publish` was to catch any error thrown while se
 // v1
 sqs.publish(queue, message);
 // v2 equivalent
-sqs.publish(queue, message, null, 'catch');
+sqs.publish(queue, message, undefined, 'catch');
 ```
 
+We encourage use of `undefined` instead of `null` if you are not using the `messageGroupId` parameter. This keeps the type simple. `null` will continue to be accepted but is deprecated as of v2, and it will be dropped from the type in v3.
+
 ## Models
 
 The `Model` base class has been removed. It's hard to make it type-safe (it tries to dynamically call setter methods) and we do modelling and validation differently now, using our [data-models](https://github.com/comicrelief/data-models) repo which is based around [Yup](https://github.com/jquense/yup).

src/services/SQSService.ts

@@ -430,15 +430,22 @@ export default class SQSService<
    * local Lambda or SQS service instead of to AWS, depending on the offline
    * mode specified by `process.env.LAMBDA_WRAPPER_OFFLINE_SQS_MODE`.
    *
-   * @param queue          string
-   * @param messageObject  object
-   * @param messageGroupId string
+   * @param queue Which queue to send to. This should be one of the queue names
+   *   configured in your Lambda Wrapper config.
+   * @param messageObject Message body to put in the queue.
+   * @param messageGroupId For FIFO queues, the message group ID. If omitted or
+   *   undefined, a random message group ID will be used.
+   *
+   *   In v1 we encouraged use of `null` if this field was unused. This is now
+   *   deprecated in favour of `undefined`. Types will still permit `null`,
+   *   however they will change in v3 to require `string | undefined`.
+   *
    * @param failureMode Choose how failures are handled:
    *   - `throw`: errors will be thrown, causing promise to reject. (default)
    *   - `catch`: errors will be caught and logged. Useful for non-critical
    *     messages.
    */
-  async publish(queue: QueueName<TConfig>, messageObject: object, messageGroupId = null, failureMode: 'catch' | 'throw' = SQS_PUBLISH_FAILURE_MODES.THROW) {
+  async publish(queue: QueueName<TConfig>, messageObject: object, messageGroupId?: string | null, failureMode: 'catch' | 'throw' = SQS_PUBLISH_FAILURE_MODES.THROW) {
     if (!Object.values(SQS_PUBLISH_FAILURE_MODES).includes(failureMode)) {
       throw new Error(`Invalid value for 'failureMode': ${failureMode}`);
     }
@@ -456,7 +463,7 @@ export default class SQSService<
 
     if (queueUrl.includes('.fifo')) {
       messageParameters.MessageDeduplicationId = uuid();
-      messageParameters.MessageGroupId = messageGroupId !== null ? messageGroupId : uuid();
+      messageParameters.MessageGroupId = messageGroupId ?? uuid();
     }
 
     try {

tests/unit/services/SQSService.spec.ts

@@ -344,7 +344,7 @@ describe('unit.services.SQSService', () => {
           sendMessage: new Error('SQS is down!'),
         }, false);
 
-        const promise = service.publish(TEST_QUEUE, { test: 1 }, null);
+        const promise = service.publish(TEST_QUEUE, { test: 1 }, undefined);
 
         await expect(promise).rejects.toThrowError('SQS is down!');
       });
@@ -354,7 +354,7 @@ describe('unit.services.SQSService', () => {
           sendMessage: new Error('SQS is down!'),
         }, false);
 
-        const promise = service.publish(TEST_QUEUE, { test: 1 }, null, SQS_PUBLISH_FAILURE_MODES.CATCH);
+        const promise = service.publish(TEST_QUEUE, { test: 1 }, undefined, SQS_PUBLISH_FAILURE_MODES.CATCH);
 
         await expect(promise).resolves.toEqual(null);
       });
@@ -364,7 +364,7 @@ describe('unit.services.SQSService', () => {
           sendMessage: new Error('SQS is down!'),
         }, false);
 
-        const promise = service.publish(TEST_QUEUE, { test: 1 }, null, SQS_PUBLISH_FAILURE_MODES.THROW);
+        const promise = service.publish(TEST_QUEUE, { test: 1 }, undefined, SQS_PUBLISH_FAILURE_MODES.THROW);
 
         await expect(promise).rejects.toThrowError('SQS is down!');
       });
@@ -377,7 +377,7 @@ describe('unit.services.SQSService', () => {
         it(`throws an error with the invalid value: ${invalidValue}`, async () => {
           const service = getService();
 
-          const promise = service.publish(TEST_QUEUE, { test: 1 }, null, invalidValue);
+          const promise = service.publish(TEST_QUEUE, { test: 1 }, undefined, invalidValue);
 
           await expect(promise).rejects.toThrowErrorMatchingSnapshot();
         });