[feature] allow sideEffects input from file instead of package.json

[bobzhang]

For a transpiler, it can make a judgement whether this file has side effect or not and emit a commit
in the end of this file, like this: https://github.com/rescript-lang/std/blob/master/lib/js/belt_List.js#L1510

The current sideEffect can only be supplied in the package.json which is inconvenient, it is
also dangerous since user's judgement is likely to be wrong.

[evanw]

I intend for esbuild to mainly follow existing conventions but to not create new ones like this. I don't indent for esbuild to be an experimentation ground for new JavaScript/package semantics. So I would only consider implementing this if it was a common convention that most other bundlers followed and that was widely used by the ecosystem.

It's a lot of effort and is a very long lead time to push out a new convention like this and get it adopted by the ecosystem. Since we already have an existing convention for this that is widely used and understood, I would prefer for the community to continue to use that single common convention instead of there being multiple ways of doing things. I suspect that having multiple conventions with similar semantics would lead to more confusion and would increase the probability that these conventions would be implemented inconsistently across bundlers.

However, I have been assuming that I will add the ability to return this flag from an esbuild plugin at some point. That is necessary to implement your own path resolution logic similar to esbuild's built-in internal path resolution logic. In that case you could write a plugin to set this flag if the file ends with that comment. That seems like an easy way to solve for this specific need.

it is also dangerous since user's judgement is likely to be wrong.

This is/was an issue with the tensorflow package. It's why there is an option to tell esbuild to ignore side effect annotations: https://esbuild.github.io/api/#tree-shaking.

[bobzhang]

Thanks for the quick reply.
The thing is that sometimes existing conventions don't follow the best practice, so it may be a good opportunity to redefine the best practice -- it is uncommon for a compiler to instrument fields in package.json, but it is common for compilers to inject some attributes for later passes.

I have been assuming that I will add the ability to return this flag from an esbuild plugin at some point.

The major selling point of esbuild is performance which rescript shares the same philosophy. I am excited that
esbuild works out of the box with rescript, but when you drag 3rd party plugins, things could be significantly slower.

[evanw]

I just asked Webpack what they think about this: webpack/webpack#12595. You should probably subscribe to that thread too. I actually agree with you that a comment directive like this would be nice. I just don't want to fragment the ecosystem if it can be avoided. My reason for wanting this feature is more that the current behavior "sideEffects" is very error-prone and can lead to accidents while a comment in the file seems like it would be much safer.

[bobzhang]

subscribed, thanks for the follow up

[kzc]

The current sideEffect can only be supplied in the package.json which is inconvenient, it is
also dangerous since user's judgement is likely to be wrong.

This is an interesting idea, but the things that immediately come to mind are:

• These comment directives may be manually inserted into source code by a user rather than a compiler, and they can also be incorrect.
• What to do when multiple comment directives in the same source file conflict? This can happen if a bundler unaware of this convention creates a library bundle with comments retained from each constituent source file.
• It can be just as awkward to annotate third party libraries' JS dist files as their package.json files. It may be better to specify the side effects as a parameter set passed to the bundler as per [QUESTION] Why is this code not tree shaked? #756 (comment).

[evanw]

Thanks for the feedback. Some thoughts:

These comment directives may be manually inserted into source code by a user rather than a compiler, and they can also be incorrect.

Incorrect annotations will be possible regardless of whichever approach is used. Incorrect parameters can also be passed to the bundler. I personally think manually authoring the annotation in the file itself instead of in the package manifest is a better approach for reasons stated in the Webpack proposal (it's more obvious when you're editing the file and it's less error-prone to the addition of new files).

What to do when multiple comment directives in the same source file conflict? This can happen if a bundler unaware of this convention creates a library bundle with comments retained from each constituent source file.

This is why I was trying to avoid having multiple ways of specifying this. I probably wouldn't even consider trying to introduce another way if I didn't think the existing way had significant drawbacks that lead to correctness issues. I imagine as a user you would only want to use one or the other but if both are specified, ideally all bundlers would behave the same (and would warn about this situation).

It can be just as awkward to annotate third party libraries' JS dist files as their package.json files.

This feature is for package authors, not for the users of packages. Ideally package authors (or a contributor) would correctly annotate their files instead of users having to do it. That way only one person would have to deal with this and their effort would be reused by everyone, instead of pushing that overhead onto everyone and everyone having to individually deal with this.

[kzc]

This feature is for package authors, not for the users of packages.

#756 (comment) is a valid use case for users of packages that do not offer a sideEffects hint, but may actually be side effect free after being vetting by the user. It is not addressed by this proposal. It would reasonable for this information to be passed to the bundler config.

[evanw]

Yup. These two features are not mutually exclusive.

[kzc]

There is a lot of overlap between the proposals so it might as well be discussed at the same time. The precedence in case of side effects conflict should probably be:

--tree-shaking=ignore-annotations > bundler config sideEffects > per file annotation > package.json sideEffects

This would allow the user to override incorrect hints in specific third party packages without resorting to disabling annotations for the entire project altogether.

[evanw]

Closing as a duplicate of #1009.