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

babel-runtime/helpers/typeof requires itself (T6644) #3711

Closed
babel-bot opened this issue Nov 17, 2015 · 28 comments
Closed

babel-runtime/helpers/typeof requires itself (T6644) #3711

babel-bot opened this issue Nov 17, 2015 · 28 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@babel-bot
Copy link
Collaborator

Issue originally made by @andy-hanson

Bug information

  • Babel version: 6.1.18
  • Node version: 5.0.0
  • npm version: 3.4.1

Options

n/a

Input code

n/a

Description

When I run npm install babel-runtime, node_modules/babel-runtime/helpers/typeof.js contains this code:

var _typeof2 = require("babel-runtime/helpers/typeof"); ... var _typeof3 = _interopRequireDefault(_typeof2); ... _typeof3.default

This is causing failures with TypeError: (0 , _typeof3.default) is not a function.

It looks like this code is itself being compiled, and needs to be changed to use the typeof operator rather than itself.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @amatiasq

And it's invoking itself recursively too :\

exports.default = function (obj) {
return obj && typeof _symbol2.default !== "undefined" && obj.constructor === _symbol2.default ? "symbol" : typeof obj === "undefined" ? "undefined" : (0, _typeof3.default)(obj);
};

_typeof3.default === exports.default

@babel-bot
Copy link
Collaborator Author

Comment originally made by @jonathanong

seeing this issue as well. i noticed that if i downgrade to babel-runtime@5, it works, but it does not work with babel-runtime@6

@babel-bot
Copy link
Collaborator Author

Comment originally made by @BowlingX

The issue is also tracked by: https://phabricator.babeljs.io/T2954

@babel-bot
Copy link
Collaborator Author

Comment originally made by Ferdinand Holzer (fholzer)

Probably related: https://github.com/fholzer/babel-issue

@babel-bot
Copy link
Collaborator Author

Comment originally made by @mairatma

Also getting this issue of typeof calling itself recursively whenever using both the es2015 preset and the transform-es2015-modules-amd plugin together, like @fholzer showed on his link above.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @timdp

I assume that the original code is in [[ https://github.com/babel/babel/blob/master/packages/babel-helpers/src/helpers.js | the typeof helper ]], which eventually falls back to a plain old typeof.

If I understand Babel's internals correctly, one of the transforms should be made aware of the fact that this is the only typeof that it shouldn't touch.

Do we know which of the transforms is breaking the code? Is it a Babel issue or a core-js one?

@babel-bot
Copy link
Collaborator Author

Comment originally made by @jevgenig

Minimalistic reproduce scenario: babel/website#612

@babel-bot
Copy link
Collaborator Author

Comment originally made by @plievone

It seems that babel-runtime/helpers/typeof.js is compiled here:
https://github.com/babel/babel/blob/2f5b953/packages/babel-runtime/scripts/build-dist.js#L61-L87
So first it takes the typeof helper template from
https://github.com/babel/babel/blob/2f5b953/packages/babel-helpers/src/helpers.js#L7-L9
Then it compiles the helper template with babel-preset-es2015 and babel-plugin-transform-runtime, so it first mangles the last typeof operator in the template due to
https://github.com/babel/babel/blob/2f5b953/packages/babel-plugin-transform-es2015-typeof-symbol/src/index.js#L12-L33
and then
https://github.com/babel/babel/blob/2f5b953/packages/babel-plugin-transform-runtime/src/index.js#L14-L18
adds the final touch of requiring the helper file itself instead of inlining the helper inside itself (not that it would be any better with inlining -- would work, but in a very convoluted way).

@babel-bot
Copy link
Collaborator Author

Comment originally made by @timdp

@plievone Many thanks for documenting this! I'd started to dig into the code myself but I never finished my write-up.

I assume there now needs to be a way to convey to transform-es2015-typeof-symbol that the typeof in question is the platform-native one rather than a polyfill? Is anyone aware of similar scenarios in the Babel source? I.e., is there another operator that's being transformed except where it falls back to the native one? That would allow us to apply an existing pattern.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @silkentrance

@timdp: +1

Looks good and can be reused for other purposes as well. Can this be integrated soon so that we can use the new babel 6.x without having to patch the typeof helper on every npm install or npm update?

@babel-bot
Copy link
Collaborator Author

Comment originally made by @timdp

I just had a stab at a fix for this issue. Please bear in mind that I'm new to Babel development. (Also, I had quite a bit of trouble getting the build to work on Windows, so that'll probably trigger a PR at some point as well.)

So. My [[ https://github.com/timdp/babel/tree/native-typeof | native-typeof branch ]] adds support for a global __babel_native__ that can be used like so:

__babel_native__("typeof", expr)

The example above gets transpiled into a native typeof expr. [[ https://github.com/timdp/babel/commit/3b9c49fbcb66de3e2cdaad268fd2dda699cae03e | This commit ]] implements the new transform and patches transform-typeof-symbol to leave its results alone.

In theory, the new transform could be extended to support any construct. It doesn't look great, but hey, it works.

Building upon this new feature, I [[ https://github.com/timdp/babel/commit/7bbf06e30464f5802514e91411082e67758961ab | replaced ]] the typeof in the helper with such an expression and I'm now getting the expected output:

exports.default = function (obj) {
  return obj && typeof _symbol2.default !== "undefined" && obj.constructor === _symbol2.default ? "symbol" : typeof obj;
};

I welcome any feedback and/or improvements.

Edit: [[ https://github.com//pull/3126 | Here's the PR. ]]

@babel-bot
Copy link
Collaborator Author

Comment originally made by @loganfsmyth

Okay, there are a ton of people subscribed to this so I'd like some feedback. Reviewing this, it seems like this may be two related by separate cases, and it's hard to say which is the one people are actually seeing.

  1. Using typeof with babel-runtime results in a (0 , _typeof3.default) is not a function error
  2. Using typeof alongside transform-es2015-modules-amd or -umd results in infinite recursion

#1 seems like it may be more user error than Babel error, but if you're running into that, I'd like to get an example repo that demos the issue, if you can. Ideally fully commit node_modules so I have the exact code to run. I'm actually not 100% clear on how to reproduce this.

#2 is a real issue that we'll resolve.

@babel-bot
Copy link
Collaborator Author

@babel-bot
Copy link
Collaborator Author

Comment originally made by @loganfsmyth

Perfect, thank you!

@babel-bot
Copy link
Collaborator Author

Comment originally made by @timdp

You don't need async/await and friends. Here's a more minimal version: https://github.com/timdp/babel-typeof-repro/tree/minimal

All it contains is transform-es2015-typeof-symbol + transform-es2015-modules-commonjs + transform-runtime. A simple typeof causes infinite recursion on my system. Doesn't seem like user error to me.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @loganfsmyth

Yup, feel free to ignore my comments about #1. It's definitely a bug. The repro case was perfect, thanks.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @loganfsmyth

I'm going to split case #2 into a separate issue, https://phabricator.babeljs.io/T6777, since this one is already primarily focused babel-runtime case.

@babel-bot
Copy link
Collaborator Author

Comment originally made by @timdp

@loganfsmyth Cool. Do you think my PR is a good solution to the loopback?

@babel-bot
Copy link
Collaborator Author

Comment originally made by @loganfsmyth

@timdp The global solution is a little iffy in my mind, but I've posted another potential fix: #3142

@babel-bot
Copy link
Collaborator Author

Comment originally made by @timdp

@loganfsmyth Yeah, I'm certainly open to suggestions. Once I figure out how your fix works, I'll let you know why I prefer it over mine. :-)

@babel-bot
Copy link
Collaborator Author

Comment originally made by @patrickheeney

Is there workaround or a patch for this issue? After looking at the potential fix repos above, I am not entirely sure where or what files to patch.

EDIT: I was able to figure out how to patch my environment based on the fix proposed by @loganfsmyth above: https://gist.github.com/patrickheeney/0e7f6324457d0c786acb .

@babel-bot
Copy link
Collaborator Author

Comment originally made by @neverfox

The PR that appears to fix this has been up for a week. This is currently a showstopper on migrating to 6, so is there anything more than needs to happen to make the PR acceptable?

@babel-bot
Copy link
Collaborator Author

Comment originally made by @loganfsmyth

This should be all set if you load babel-runtime@6.3.19.

@babel-bot
Copy link
Collaborator Author

Comment originally made by Jonathan (barroudjo)

Just to let you know, I still have the TypeError: (0 , _typeof3.default) is not a function error when using babel-plugin-tranform-runtime (which requires babel-runtime).

@sheerun
Copy link

sheerun commented Oct 24, 2016

It still happens in babel-runtime 6.11.6 when using rollup, please reopen:

A module cannot import itself (.../node_modules/babel-runtime/helpers/typeof.js)

@sheerun
Copy link

sheerun commented Oct 24, 2016

This is rollup.config.js to reproduce:

import json from 'rollup-plugin-json';
import babel from 'rollup-plugin-babel';
import commonjs from 'rollup-plugin-commonjs';
import nodeResolve from 'rollup-plugin-node-resolve';

export default {
  entry: 'node_modules/babel-runtime/helpers/typeof.js',
  format: 'cjs',
  plugins: [
    json(),
    babel({ runtimeHelpers: true }),
    nodeResolve({ jsnext: true, main: true }),
    commonjs()
  ],
  dest: 'bundle.js'
};

@arbesfeld
Copy link

@sheerun, did you come up with a workaround for this?

@sheerun
Copy link

sheerun commented Nov 10, 2016

@arbesfeld No, I've opened a ticket on in #4829

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 6, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

3 participants