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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comment-related warnings when building with Vite #1916

Closed
branko-d opened this issue Dec 4, 2023 · 3 comments 路 Fixed by #1919
Closed

Comment-related warnings when building with Vite #1916

branko-d opened this issue Dec 4, 2023 · 3 comments 路 Fixed by #1919

Comments

@branko-d
Copy link

branko-d commented Dec 4, 2023

馃悰 Bug report

Current Behavior

When building a production bundle with Vite, I get a bunch of warnings like this:

A comment

"/*#__PURE__*/"

in "node_modules/fp-ts/es6/Either.js" contains an annotation that Rollup cannot interpret due to the position of the comment. The comment will be removed to avoid issues.

Expected behavior

No build warnings.

Reproducible example

  1. Go here: https://stackblitz.com/edit/vitejs-vite-y7ddkf?file=src%2FApp.tsx
  2. This is just a standard Vite's react-ts example with "fp-ts" added to package.json and imported in App.tsx.
  3. Click on the Terminal panel at the bottom.
  4. Enter q + ENTER to quid Vite dev server.
  5. Type: vite build.

You should be able to see the warnings in the build log.

Suggested solution(s)

Not sure - probably fix the comments somehow?

Your environment

Which versions of fp-ts are affected by this issue?

Software Version(s)
fp-ts 2.16.1
TypeScript 5.2.2

Did this work in previous versions of fp-ts?

Not sure - we just transitioned away from Create React App (where we didn't get these warnings) to Vite recently.

kblcuk added a commit to kblcuk/fp-ts that referenced this issue 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 gcanti#1916

[0] https://rollupjs.org/configuration-options/#pure
@kblcuk
Copy link
Contributor

kblcuk commented Jan 3, 2024

FIY I stumbled upon the same issue and after some investigation I think I might have a fix 馃

gcanti pushed a commit that referenced this issue 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
@cfgs
Copy link

cfgs commented Jan 23, 2024

FIY I stumbled upon the same issue and after some investigation I think I might have a fix 馃

How is the fix going? :)

@kblcuk
Copy link
Contributor

kblcuk commented Jan 23, 2024

@cfgs you can see a bit above your comment that this issue was closed in #1919 :)

So once you update to the latest fp-ts, the warnings should disappear for you as well!

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 a pull request may close this issue.

3 participants