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

'this' becomes 'undefined' in es2015 preset #7636

Closed
thany opened this issue Mar 27, 2018 · 15 comments

Comments

Projects
None yet
5 participants
@thany
Copy link

commented Mar 27, 2018

Bug: when using this at the global level of your js code, using the es2015 or env preset, it will be transformed to undefined, which is not correct.

Input Code

(function($) {
  // ...
}(this.jQuery));

Output

"use strict";

(function ($) {
  // ...
})(undefined.jQuery);

Babel Configuration

Calling babel via gulp:

.pipe(babel({
    presets: [ "env" ]
}))

No config file otherwise.

Expected Behavior

this should not be transformed into undefined. There is no reason to.

Current Behavior

this at the global context is transformed into undefined.

Possible Solution

It works fine in the es2017 preset, so perhaps copy over what that preset is doing, into es2015.

Context

I'm trying to transform modern javascript into javascript that oldIE can understand, and the above code is how we write our top-level IIFE to avoid conflicts in the global context. this in the global context means the window object.

Your Environment

software version(s)
Babel 6.26.0
Babel-preset-env 1.6.1
Babel-preset-es2015 6.24.1
Gulp-babel 7.0.1
node 8.10.0
npm 5.7.1
Operating System Windows 10

Noet: the issue can also be reproduced on the Try it out thing.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Mar 27, 2018

Hey @thany! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Mar 27, 2018

In ES modules global this is undefined. You can disable the modules transform by passing "modules": false to the es2015/env presets.

@thany

This comment has been minimized.

Copy link
Author

commented Mar 28, 2018

I'm not so sure about that actually. The global this is imposed by the browser, and cannot be overridden by any script, however crafty. That is the reason I'm using this in favour of window - it's always available. this can only be undefined in a function context where that function is explicitly called like fn.call(undefined, ...) but this doesn't and cannot happen for the global context.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Mar 28, 2018

The global this is (and must be) always undefined in modules. Since the es2015 preset transpiles modules, it must set this = undefined. If you don't want this behavior, you need to diable the modules transformer.

@loganfsmyth

This comment has been minimized.

Copy link
Member

commented Mar 28, 2018

The global this is (and must be) always undefined in modules.

I don't think that's a good way to think about it. The module-level execution environment, which is a level above the global scopes, specifically sets this to undefined, which means the global this is still the same, but it is shadowed.

https://www.ecma-international.org/ecma-262/8.0/#sec-module-environment-records-getthisbinding

@thany

This comment has been minimized.

Copy link
Author

commented Mar 28, 2018

@nicolo-ribaudo
Alright I guess, if you want that to be the default.

But that still doesn't answer the question: why replace this with undefined? It'll be undefined anyway when using modules, apparently. And when not using modules, it'll be what the browser intents it to be.

Why does anyone need this, which is undefined, be replaced by an explicit undefined? Your explanation suggests that leaving this as-is, will impose no functional difference in either case.

@loganfsmyth
That's what I was thinking. The undefined this cannot ever be the actual global this. It will be a function-scoped this. Having said that, Babel probably has no reliable way of knowing whether a file is a module or not, by just looking at it. Hence my suggestion to simply leave it as-is.

@loganfsmyth

This comment has been minimized.

Copy link
Member

commented Mar 28, 2018

But that still doesn't answer the question: why replace this with undefined? It'll be undefined anyway when using modules, apparently.

This behavior happens in transform-es2015-modules-commonjs, and the this binding in CommonJS modules is the exports object, but undefined. If we left them as-is, it would have the wrong value.

Having said that, Babel probably has no reliable way of knowing whether a file is a module or not, by just looking at it. Hence my suggestion to simply leave it as-is.

Right now Babel simply assumes that the file is an ES module. I 100% agree that this is not ideal, and we're changing that in Babel 7.

@thany

This comment has been minimized.

Copy link
Author

commented Mar 29, 2018

and the this binding in CommonJS modules is the exports object, but undefined. If we left them as-is, it would have the wrong value.

How would it be incorrect to leave it as-is? How could that possibly be? If this is undefined, leaving this as this means that it'll still be undefined, as imposed by the module system. If you replace it, nothing changes. Why does Babel even care in the first place?

@loganfsmyth

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

the this binding in CommonJS modules is the exports object, but undefined

Sorry that but is a typo, it should be not. this in CommonJS modules is the exports object. exports === this

@thany

This comment has been minimized.

Copy link
Author

commented Mar 29, 2018

Ah, I see. Well then it should definitely not be replaced. Just leave it as-is so a script can use it as the exports object or the window global, whichever system the script is designed for. Replacing it with undefined defeats both scenarios. Not replacing it seems to only reduce the need for the modules option, which feels like a good thing.

@loganfsmyth

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

Like I said, we're working on this in Babel 7. If a file isn't marked as an ES module, this transform won't happen. If you want to use this as exports then you can't use import and export syntax.

@thany

This comment has been minimized.

Copy link
Author

commented Mar 29, 2018

I get that, but I'm saying this transform doesn't ever need to happen.

@TrySound

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2018

@thany It need to happen with import and export syntax.

@loganfsmyth

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

Yeah, that is not the case. ES modules explicitly see this as undefined, and if we skipped this transform and people converted the ES modules to CommonJS, this would then be the exports object, which would not be spec-compliant behavior for ES modules.

@thany

This comment has been minimized.

Copy link
Author

commented May 2, 2018

Whaterver ES modules see this as, is not relevant. A minifier should NEVER change the meaning of code. So transforming this to undefined is frankly just bonkers.

@lock lock bot added the outdated label Aug 1, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.