Emit 'use strict' directive for cjs builds

[lxsmnsyc]

Bundled CommonJS build formats should be able to emit a 'use strict` directive at the beginning of the file.

[evanw]

Can you say more about your use case? Why would you expect this? Are all, some, or none of the input files marked as "use strict"?

[lxsmnsyc]

It's for package distribution, such as preventing global access to unknown identifiers.

Personally, I thought the alwaysStrict field for tsconfig works with ESBuild, but I don't really expect ESBuild to be 100% compatible with the options.

I tried adding the directive for every module but it fails as I expected.

[rtsao]

In terms of correctness, omitting strict mode directives when compiling to CJS can affect runtime behavior. I'm a bit skeptical this would matter for any real-world use case, but a simple contrived example is:

// index.mjs
export function foo() { return arguments.callee; }

export function log(fn) {
  try {
    fn();
    console.log("success");
  } catch (e) {
    console.log("failure");
  }
}

log(foo);

Because modules are implicitly strict mode, this will log failure but after bundling to CJS will log success.

[lxsmnsyc]

@rtsao assuming that the modules are not treated as externals, then yes, the usual behavior would be different. As the goal for this is for package distribution of the resulting bundle and not just bundling alone, I think the inclusion of the directive would be helpful.

[evanw]

In terms of correctness, omitting strict mode directives when compiling to CJS can affect runtime behavior.

In terms of correctness, inserting strict mode directives can also break code. I think the principled thing to do would be to only put a strict mode directive if all files had it. This seems simple to do to me. I could also see just respecting alwaysStrict in tsconfig.json being a reasonable solution.

Otherwise, if some files were mixed, individual files marked as strict mode would need to be put in a closure. But then that would mess with bundling (e.g. potentially adding bloat and disabling tree shaking) as well as potentially making bundling much more complicated if there are additional workarounds added to try to do better in these cases, which doesn't sound great. So I'm not sure if doing that is the right thing here.

I wonder what other bundlers do.

[rtsao]

Yeah, I think webpack will only add "use strict" for modules that are strict mode (at the expense of less scope hoisting and more function wrappers). Definitely some additional complexity and bloat, but it AFAIK there won't be any situations where webpack introduces strict mode-related breakages. In the past, I've seen apps using webpack that depended on certain node_modules that break in strict mode. Realistically, I think this scenario is considerably more likely than the presence of code that would break because it is not strict mode when it should be.

[lxsmnsyc]

In terms of correctness, inserting strict mode directives can also break code. I think the principled thing to do would be to only put a strict mode directive if all files had it.

This seems good to me, although adding them manually is a bit of a different experience for me (I have never manually included the directive because the compilers does it for me). Tbh, my packages before used Rollup, and I turned to ESBuild for faster build times. I just found out recently that one of the differences between the two is the lack of 'use strict' directives, which is the contrary in webpack and bucklescript.

Here's an example of before and after using ESBuild,

before: https://unpkg.com/@lxsmnsyc/react-scoped-model@1.4.3/dist/react-scoped-model.cjs.development.js
after: https://unpkg.com/@lxsmnsyc/react-scoped-model@1.5.2/dist/react-scoped-model.development.js

[evanw]

This should now be possible with the banner feature: https://esbuild.github.io/api/#banner. I'm going to close this issue since I think using the banner feature is an acceptable solution.

[lxsmnsyc]

This should now be possible with the banner feature: https://esbuild.github.io/api/#banner. I'm going to close this issue since I think using the banner feature is an acceptable solution.

This could be a decent workaround, thank you @evanw