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 missing pure annotations #175

Merged
merged 1 commit into from
Jan 17, 2022
Merged

Add missing pure annotations #175

merged 1 commit into from
Jan 17, 2022

Conversation

OliverJAsh
Copy link
Contributor

@OliverJAsh OliverJAsh commented Jan 14, 2022

We also need to add pure annotations to the classes defined in index.js, however we can't add the pure annotation in the correct position because these are transpiled to IIFEs.

For example:

export class Setter<S, A> {

This will become:

var Setter = /** @class */ (function () {
    function Setter() {
    }
    return Setter;
}());
export { Setter };

The pure annotation needs to be added like so:

-var Setter = /** @class */ (function () {
+var Setter = /** @class */ /*#__PURE__*/ (function () {

I believe this can be solved by using a Babel plugin that adds the pure annotation to output of TypeScript. Alternatively, we can disable class transpilation.


Pure annotations are very easy to miss and hard to get right, so perhaps we should automate this across the fp-ts ecosystem, using something like one of these tools. WDYT @gcanti?

@gcanti gcanti merged commit ee42bff into gcanti:master Jan 17, 2022
@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Jan 20, 2022

I believe this can be solved by using a Babel plugin that adds the pure annotation to output of TypeScript.

Pure annotations are very easy to miss and hard to get right, so perhaps we should automate this across the fp-ts ecosystem, using something like one of these tools. WDYT @gcanti?

@gcanti Have you tried to do anything like this before? If not then I can take a look.

@gcanti
Copy link
Owner

gcanti commented Jan 20, 2022

@OliverJAsh No I haven't, I was afraid to mess up the tsc output

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.

None yet

2 participants