fix: create server req body spread operator

[bartoszherba]

Description

In revision 1558c3242bfa5aca9207ef5e41839cbee558cd8e we changed the TS compiler target from es5 to es2019 which completely changed the compiled output. In the previous version, the result aligned with the spread operator intentions ...req.body but in the current, non-iterable objects yield error Found non-callable @@iterator as spread can be used only on iterable objects like arrays or strings. To address this issue I added the check to eventually wrap non-iterable objects in the array.

Please compare compiled versions below.

target ES2019 (current)

app.post('/:integrationName/:functionName', async (req, res) => {
        const { integrationName, functionName } = req.params;
        const { apiClient, configuration, extensions, customQueries, initConfig } = integrations[integrationName];
        const middlewareContext = { req, res, extensions, customQueries };
        const createApiClient = apiClient.createApiClient.bind({ middleware: middlewareContext });
        const apiClientInstance = createApiClient({ ...configuration, ...initConfig });
        const apiFunction = apiClientInstance.api[functionName];
        try {
            const platformResponse = await apiFunction(...req.body);
            res.send(platformResponse);
        }
        catch (error) {
            res.status(getAgnosticStatusCode(error));
            res.send(error);
        }
    });

target "target": "es5"

app.post('/:integrationName/:functionName', function (req, res) { return __awaiter(_this, void 0, void 0, function () {
        var _a, integrationName, functionName, _b, apiClient, configuration, extensions, customQueries, middlewareContext, createApiClient, apiClientInstance, apiFunction, platformResponse, error_1;
        return __generator(this, function (_c) {
            switch (_c.label) {
                case 0:
                    _a = req.params, integrationName = _a.integrationName, functionName = _a.functionName;
                    _b = integrations[integrationName], apiClient = _b.apiClient, configuration = _b.configuration, extensions = _b.extensions, customQueries = _b.customQueries;
                    middlewareContext = { req: req, res: res, extensions: extensions, customQueries: customQueries };
                    createApiClient = apiClient.createApiClient.bind({ middleware: middlewareContext });
                    apiClientInstance = createApiClient(configuration);
                    apiFunction = apiClientInstance.api[functionName];
                    _c.label = 1;
                case 1:
                    _c.trys.push([1, 3, , 4]);
                    return [4 /*yield*/, apiFunction.apply(void 0, req.body)];
                case 2:
                    platformResponse = _c.sent();
                    res.send(platformResponse);
                    return [3 /*break*/, 4];
                case 3:
                    error_1 = _c.sent();
                    res.status(getAgnosticStatusCode(error_1));
                    res.send(error_1);
                    return [3 /*break*/, 4];
                case 4: return [2 /*return*/];
            }
        });
    }); });

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots:

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] I have read the CONTRIBUTING document.

Changelog

• [ ] I have updated the Changelog (V1) v2 and mentioned all breaking changes in the public API.
• [ ] I have documented all new public APIs and made changes to existing docs mentioning the parts I've changed so they're up to date.

Tests

• [ ] I have written test cases for my code
• [ ] I have tested my Pull Request on production build and (to my knowledge) it works without any issues
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

  I tested manually my code, and it works well with both:

• [ ] Default Theme
• [ ] Capybara Theme

Code standards

• [ ] My code follows the code style of this project.
• [ ] I have followed naming conventions

Docs

• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.

[bartoszherba]

@bartoszherba Looking at the logic in createServer.ts: can't we just remove the spread operator and pass a plain req.body reference as the only argument to the apiFunction?

Perhaps the intention of the author here was to prevent the apiFunction from altering the req.body object. However, I think it should rather be a matter for a well-designed apiFunction which should not cause any side effects and stay pure.

If we pass req.body as the only argument here, we will get rid of the problematic spread operator as well as avoid an additional check preventing us from spreading a non-iterable entity.

WDYT?

I did it like I did because I wanted to preserve the backward compatibility with any external integration or modules that we have. If I change the argument of the function I cannot guarantee that. It was just too risky. Also, if we can prevent mutation of the input from the caller then I think it is better and safer. We cannot assume that the input will be not modified if it mustn't be modified, though we can prevent it.

[lsliwaradioluz]

@bartoszherba Totally agree, I've made some wrong assumptions while looking at this logic, mainly due to the fact that I have a memory of this part of our docs about extending API methods.

It suggests that every API method can only receive context and params as arguments whereas in fact it could even be context, param1, param2, paramN - depending on how we call this API method.

This PR is approved.