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

"use strict" not being emitted automatically if needed breaks code #2264

Closed
fabiospampinato opened this issue May 24, 2022 · 7 comments
Closed

Comments

@fabiospampinato
Copy link

fabiospampinato commented May 24, 2022

Long story short esbuild is not emitting "use strict"; while Vite is, even if I point it to my "tsconfig.json" with "alwaysStrict" in it.

IMO this is a really bad default that breaks code in subtle ways.

For my sample project I'm basically getting this with esbuild:

function self () {
  return this;
}

const bind = self.bind.bind ( self );
const value = bind ( '123' )();

console.log ( value );
console.log ( typeof value );

But this with Vite:

'use strict';

function self () {
  return this;
}

const bind = self.bind.bind ( self );
const value = bind ( '123' )();

console.log ( value );
console.log ( typeof value );

And for whatever insane reason outside of strict mode this code outputs a String object, while in strict mode it outputs a string primitive.

Outputting the strict mode directive IMO is just a much better default, nobody should be using sloppy mode anyway really.

Also it's unclear how well the banner workaround works, potentially I could be using multiple plugins each setting their own strict mode directive? Maybe some other plugin could override my plugin's request to use strict mode? It just doesn't make a lot of sense to me to use banner for this. If anything this should be opt-out not opt-in.

Sample project:

voby_bug.zip

@hyrious
Copy link

hyrious commented May 25, 2022

It is rollup (behind vite) emitting this line by default. The reason for this behavior is that the input format for rollup/vite are always ESM, where there's only strict mode.

On the other hand, esbuild can handle non-strict code if it is not told to be strict (without use strict, or not in ESM format). It won't show error on let a = 00.

Maybe we could ask esbuild to emit that banner automatically if strict mode is enabled in one of the source code (when it see use strict or code is in ESM format). One downside is that we cannot preserve non-strict code behavior any more, which may break some aged libraries.


It seems that esbuild will add a top-level use strict when it sees one in the source code, yet not removing the original one in esm sources:

'use strict'
export let a = 1

esbuild a.js --bundle --format=esm --minify

"use strict";var t=1;"use strict";export{t as a};
//                   ^~~~~~~~~~~~
// not shown in CJS or IIFE format

The same output presents in transform mode.
CJS input + IIFE output seems to be the same.

@fabiospampinato
Copy link
Author

fabiospampinato commented May 25, 2022

The --banner workaround (--banner:js='\"use strict\";') seems to get the job done at least though.

If I write "use strict"; manually in the entry point of my library I get 2 "use strict"; in the output though.

@evanw
Copy link
Owner

evanw commented May 26, 2022

I think there are a few different things here:

  • The "use strict" directive is duplicated twice in ESM output. This is just an oversight. I didn't think to test this because "use strict" is irrelevant in this case (since ESM always implies strict mode). Arguably it shouldn't ever be inserted at all for ESM output. It's easy to fix the duplicate one, so I can do that.

  • There is a request for "use strict" to always be generated. I disagree that this is a good idea. Right now the presence of "use strict" in the output is determined by the presence of "use strict" in the entry point file. I think that's the most straightforward way to control this behavior, so I'm planning to keep it this way.

  • There is a request for esbuild to respect alwaysStrict in tsconfig.json. It's not expected that this should work because esbuild only supports a subset of tsconfig.json options: https://esbuild.github.io/content-types/#tsconfig-json. But that's a reasonable request, and I can see it making sense for esbuild to support this option.

@curiousdannii
Copy link

curiousdannii commented Jun 24, 2022

I was surprised to see "use strict" added to my builds and couldn't work out what had changed until I saw this in the changelog. "use strict" shouldn't be added if the format is ESM, because ES modules are always strict.

Removing a "use strict" that's directly from the source file is probably going too far though.

@fabiospampinato
Copy link
Author

@curiousdannii Are you seeing a "use strict" being added to your build as a result of having the "alwaysString" option in tsconfig.json?

The problem with reverting this change is that you as the library author are assuming that your code is running under strict mode, because you are outputting an ESM module, but once the user gets to actually execute your code that may no longer be the case, depending on the bundler used, I guess.

@curiousdannii
Copy link

curiousdannii commented Jun 24, 2022

Yes, "use strict" started to be added because my tsconfig.json has "strict": true.

If someone is doing additional bundling after esbuild, then it's their responsibility to ensure they're bundling it correctly. I don't see why esbuild should add an unnecessary pragma just in case someone bundles it incorrectly. If esbuild tried to protect against anything that could possibly be misconfigured then it would cease being a very efficient minifier!

Though I'm saying that more as an app author than a library author. Maybe things might be slightly different from the perspective of a library author? Not sure. Does esbuild currently add "use strict" pragmas when ES modules are embedded into a CJS/IIFE bundle? That would make sense.

@unilynx
Copy link

unilynx commented Feb 9, 2023

I'm running into an issue which seems to be related to this (or might not be?)

i'm switching from tsc to esbuild (actually, a fork of esbuild-runner to replace ts-node) and a test that worked under ts-node now fails because esbuild isn't emitting "use strict" - which causes frozen (Object.freeze) properties to no longer throw.

I'm unclear as to whether adding strict/alwaysStrict to tsconfig should have fixed it. If it should, I'll investigate further.

For now the banner workaround in #2264 (comment) fixes this case for me, but it feels a bit ugly :)

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

No branches or pull requests

5 participants