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

Working around another Babel-core 6.17.0 transpilation error in iffElse #89

Merged
merged 1 commit into from
Jan 7, 2017

Conversation

eddyystop
Copy link
Collaborator

@eddyystop eddyystop commented Jan 6, 2017

  • Only new module changed as old will be deleted soon.
  • They caused the wrong 'this' being passed to predicate and hooks.
  • @daffl says transpile is OK in Babel-core 6.21.0
  • Added tests to catch any problems in predicate and hooks
  • Babel-core 16.7.0 transpiles
    const check = typeof predicate === 'function' ? predicate(...fnArgs) : !!predicate;
    as
    var check = typeof predicate === 'function' ? predicate.apply(undefined, fnArgs) : !!predicate;

- They caused the  wrong 'this' being passed to predicate and hooks.
- @daffl says transpile is OK in Babel-core 6.21.0
- Added tests to catch any problems in predicate and hooks
- Babel-core 16.7.0 transpiles
const check = typeof predicate === 'function' ? predicate(...fnArgs) : !!predicate;
as
var check = typeof predicate === 'function' ? predicate.apply(undefined, fnArgs) : !!predicate;
@daffl
Copy link
Member

daffl commented Jan 6, 2017

What should the context be? In strict mode, the context of a function should be undefined when called like this. I'd also try and use the latest versions (clear cache and reinstall) in case it was a bug that has been fixed. I think Babel is around 6.20 now.

@daffl
Copy link
Member

daffl commented Jan 6, 2017

I'm mainly asking because we might be fixing a non-issue. E.g. when using arrow functions at the module level scope, the context should always be undefined (which is why I don't like using top level arrow functions).

@eddyystop
Copy link
Collaborator Author

The containing function is a top-level arrow functions. Thanks!

I have a bunch of PRs I'd like to push in order to avoid merge conflicts. So I suggest we push this PR for its tests and then I'll confirm the issue's with the top-level being an arrow function, as the details are a bit more complicated.

@daffl
Copy link
Member

daffl commented Jan 6, 2017

Sounds good. 👍

@ekryski
Copy link
Member

ekryski commented Jan 6, 2017

Sounds good. :shipit: Let us know if that is still in an issue with Babel v6.20.

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

Successfully merging this pull request may close these issues.

3 participants