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

[TIMOB-25806] iOS: Fix arrow-functions in Hyperloop #272

Merged
merged 1 commit into from Mar 16, 2018
Merged

Conversation

hansemannn
Copy link
Contributor

@hansemannn hansemannn commented Feb 24, 2018

JIRA: https://jira.appcelerator.org/browse/TIMOB-25806

We may add more tests for other ES6 styles, like rest parameters.

@hansemannn hansemannn added this to the v3.1.0 milestone Feb 24, 2018
@hansemannn hansemannn changed the title [TIMOB-25806] iOS: Fix arrow-functions in Hyperloop and ES6 [TIMOB-25806] iOS: Fix arrow-functions in Hyperloop Feb 24, 2018
Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is correct, but we probably also need to support 'FunctionDeclaration'.

According to estree, the possible AST node types for a function include that as well: https://hexdocs.pm/estree/ESTree.Function.html#content

@hansemannn
Copy link
Contributor Author

I couldn't find the FunctionDeclaration type in the AST-parser we use (and the logs I tried). Maybe move this to your ES6 PR?

@sgtcoolguy
Copy link
Contributor

A function declaration is simply a function defined standalone, versus being part of some other statement/assignment:

declaration:

function myFunc() {
  // whatever
}

expression:

var myFunc = function () {
  // whatever
};

more: https://javascriptweblog.wordpress.com/2010/07/06/function-declarations-vs-function-expressions/

@hansemannn
Copy link
Contributor Author

There is still no FunctionDeclaration type in the AST-parser library from what I see, so should we handle it manually?

@sgtcoolguy
Copy link
Contributor

@sgtcoolguy
Copy link
Contributor

Ah, now I see why you'd never run into it - the way this is called, I think is typically on the right-hand side of a variable declaration. I suppose if we run into the case we can add it.

@sgtcoolguy sgtcoolguy merged commit e500fa7 into master Mar 16, 2018
@hansemannn
Copy link
Contributor Author

hansemannn commented Mar 16, 2018

Ok cool! There is still some weird issue with the scope, where this.buttonPressed would not be found:

    var ButtonDelegate = Hyperloop.defineClass('ButtonDelegate', 'NSObject');

    ButtonDelegate.addMethod({
        selector: 'buttonPressed:',
        instance: true,
        arguments: ['UIButton'],
        callback: (sender) => {
            // Not existing with ES6 enabled
            if (this.buttonPressed) {
                this.buttonPressed(sender);
            }
        }
    });

    var delegate = new ButtonDelegate();
   
    // Not called with ES6 enabled
    delegate.buttonPressed = function(sender) {
        alert('Button pressed!');
    };

Its a different kind of issue, so maybe we need to restructure the example to not use this kind of "method-assignments".

Funny enough: If the delegate is in a different file like here, it works. It's tracked in TIMOB-25807.

@hansemannn hansemannn deleted the TIMOB-25806 branch April 12, 2018 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants