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

Fix __PURE__ annotation placement #1919

Merged
merged 2 commits into from Jan 3, 2024

Conversation

kblcuk
Copy link
Contributor

@kblcuk kblcuk commented Jan 3, 2024

Currently there are some arrow functions that have /#PURE/
annotation before function call to identify that that function call is
side effect free.

However since this project is built to es5 target, which doesn't have
arrow functions support, the code is then translated into regular
function that loos something like this:

/** @internal */
export var flatMapNullable = function (F, M) {
    /*#__PURE__*/ return dual(3, function (self, f, onNullable) {
        return M.flatMap(self, liftNullable(F)(f, onNullable));
    });
};

However this makes some built tools (Vite (which uses Rollup under the
hood) in our case) unhappy, and build produces a lot of warnings about
that PURE annotation being in the wrong place.

Checking the Rollup docs [0], it seems that pure annotation should be
placed right before function invocation, so in this particular case
between return keyword and the actual function.

So this simply changes arrow functions in question to have explicit
return keyword and annotation before the function call, which leaves no
interpretation to ts compiler, and makes our code build process
warning-free.

This also potentially fixes #1916

[0] https://rollupjs.org/configuration-options/#pure

Currently there are some arrow functions that have /*#__PURE__*/
annotation before function call to identify that that function call is
side effect free.

However since this project is built to es5 target, which doesn't have
arrow functions support, the code is then translated into regular
function that loos something like this:

```
/** @internal */
export var flatMapNullable = function (F, M) {
    /*#__PURE__*/ return dual(3, function (self, f, onNullable) {
        return M.flatMap(self, liftNullable(F)(f, onNullable));
    });
};
```

However this makes some built tools (Vite (which uses Rollup under the
hood) in our case) unhappy, and build produces a lot of warnings about
that __PURE__ annotation being in the wrong place.

Checking the Rollup docs [0], it seems that pure annotation should be
placed right before function invocation, so in this particular case
between `return` keyword and the actual function.

So this simply changes arrow functions in question to have explicit
return keyword and annotation before the function call, which leaves no
interpretation to ts compiler, and makes our code build process
warning-free.

This also potentially fixes gcanti#1916

[0] https://rollupjs.org/configuration-options/#pure
It should be placed before function invocation, but here we're just
mapping one function to another and casting it to `any`.
@gcanti gcanti merged commit 16bc70b into gcanti:master Jan 3, 2024
1 check passed
@gcanti
Copy link
Owner

gcanti commented Jan 3, 2024

Thank you @kblcuk, going to release this asap

@kblcuk kblcuk deleted the fix-pure-annotation-placement branch January 3, 2024 16:29
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.

Comment-related warnings when building with Vite
2 participants