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

Can't Extend Native Classes with ES6 #818

Open
NickIliev opened this issue Nov 6, 2017 · 29 comments
Open

Can't Extend Native Classes with ES6 #818

NickIliev opened this issue Nov 6, 2017 · 29 comments
Labels
bug

Comments

@NickIliev
Copy link
Member

@NickIliev NickIliev commented Nov 6, 2017

From @vbresults on November 6, 2017 11:22

I ran into this when setting a delegate in my project which is TypeScript compiled down to ES6. The problem would be that application.js expects a native delegate object while it's a JS function in ES6.

Copied from original issue: NativeScript/NativeScript#5038

@ginev

This comment has been minimized.

Copy link
Contributor

@ginev ginev commented Nov 20, 2017

@vbresults can we have a snippet that we can use to reproduce the case?

@swlemp

This comment has been minimized.

Copy link

@swlemp swlemp commented Nov 20, 2017

class Delegate extends UIResponder implements UIApplicationDelegate {
	public static ObjCProtocols = [UIApplicationDelegate];

	applicationDidReceiveRemoteNotification(application: UIApplication, userInfo: NSDictionary<string, any>) {
		// ...<snip>
	}

	// ...<snip>
}

const application = require("application");
application.ios.delegate = Delegate;
@tdermendjiev

This comment has been minimized.

Copy link
Collaborator

@tdermendjiev tdermendjiev commented Nov 20, 2017

Hi @vbresults, are you getting any errors while building/running?

@ginev

This comment has been minimized.

Copy link
Contributor

@ginev ginev commented Nov 20, 2017

@vbresults - this looks like it should be working as expected. The application.ios property returns an instance of the IOSApplication class which we internally use to represent an iOS app. Its delegate property accepts a type, not an instance of it - so you basically need to set it to point to the class which instance you would like to use as a custom delegate.

This article might be helpful in resolving the case: https://docs.nativescript.org/core-concepts/application-lifecycle#ios-uiapplicationdelegate

I don't see you calling the start() method of the app. Can you share some more code here?

@swlemp

This comment has been minimized.

Copy link

@swlemp swlemp commented Nov 20, 2017

@tdermendjiev Yes, the error is that Delegate is not a native class but instead a JS function.

@tdermendjiev

This comment has been minimized.

Copy link
Collaborator

@tdermendjiev tdermendjiev commented Nov 20, 2017

@vbresults it would be useful if you provide the exact error with the stack trace as well as the whole content of the js file.

@swlemp

This comment has been minimized.

Copy link

@swlemp swlemp commented Nov 21, 2017

@tdermendjiev It's triggered by this line in application.ios.js:

UIApplicationMain(0, null, null, iosApp && iosApp.delegate ? NSStringFromClass(iosApp.delegate) : NSStringFromClass(Responder));

NSStringFromClass expects a class but instead it's getting a JS function. My guess is that NS uses some sort of built-in __extends function which does not affect ES6+ native extends.

Make sure lib is set to es6 and dom in tsconfig.json.

@tdermendjiev

This comment has been minimized.

Copy link
Collaborator

@tdermendjiev tdermendjiev commented Nov 21, 2017

@vbresults I managed to reproduce the issue and indeed its related to the __extends function implemented by the ios runtime. ES6 extends has a different behavior and it doesn't call runtime's __extends. That said, ES6 is not supported by both the iOS and Android runtimes. We don't plan supporting ES6 in the near future.

@swlemp

This comment has been minimized.

Copy link

@swlemp swlemp commented Nov 21, 2017

@tdermendjiev That's unfortunate. Native ES6 does not have some of the limitations that __extends does (specifically around inheritance and calling context); ES6 really helps to clean up my code.

@ginev

This comment has been minimized.

Copy link
Contributor

@ginev ginev commented Nov 21, 2017

@vbresults ES6 support will certainly become more demanded and we will have to support it at one point. By saying that we do not plan it for the near future our point is that we do not plan it for the upcoming two or three releases of NativeScript. If the demand for it increases, we will escalate its implementation.

@etabakov etabakov added the bug label Mar 28, 2018
@etabakov etabakov changed the title Bug: Can't Extend Native Classes with ES6 Can't Extend Native Classes with ES6 Mar 28, 2018
@louishfok

This comment has been minimized.

Copy link

@louishfok louishfok commented Jul 19, 2018

+1 es6+ is definitely where the industry is going. Any updates on this?

@bradmartin

This comment has been minimized.

Copy link

@bradmartin bradmartin commented Sep 22, 2018

Ran into this tonight. With the recent updates to the android runtime to use the updated V8 versions. I've been running many apps with the tsconfig outputting es7 to use native async/await. Tonight I was working on updating an app that implements a delegate for the social-login plugin and the app crashed on start up and led me here. So with android supporting es7 I'd definitely think the iOS side should be moving in a forward direction.

Is the issue in the JSCore side? Curious how that works 😄

@mbektchiev

This comment has been minimized.

Copy link
Collaborator

@mbektchiev mbektchiev commented Sep 28, 2018

I have both good and bad news regarding this issue. The good one is that I managed to come up with a workaround for now, and bad is that we still haven't planned to invest time in making it work automatically.

const Del = (UIResponder as any).extend({
    applicationDidBecomeActive(application: UIApplication): void {
        console.log("applicationDidBecomeActive", application);
    }
}, {
    protocols: [UIApplicationDelegate]
});

import * as application from 'application';

application.ios.delegate = Del;

application.run({ moduleName: 'app-root' });

This snippet casts the base class to any and calls the provided by NativeScript extend function. This way you can use ES2015 target for all other classes and at the same time circumvent the limitation that native classes cannot be subclassed. I hope this way you'll be able to switch to "target": "es2015".

One of the approaches for adding such support that I was thinking of was to provide a function attached to the global object, that has to be called manually for each class which extends a native class. For example something like this:

class Delegate extends UIResponder implements UIApplicationDelegate {
	public static ObjCProtocols = [UIApplicationDelegate];

	applicationDidReceiveRemoteNotification(application: UIApplication, userInfo: NSDictionary<string, any>) {
		// ...<snip>
	}

	// ...<snip>
}

__ns_extend_native(Delegate, UIResponder);

Maybe we can log a new feature request for that, if you think that this idea is good.

@NathanWalker

This comment has been minimized.

Copy link

@NathanWalker NathanWalker commented Nov 18, 2018

@mbektchiev helpful idea - I just came across this today as well. I like your suggestion however perhaps instead of a rather obj-c'y kind of name like __ns_extend_native perhaps could be more inline with typical JS framework styles and consistent with {N} itself.

There's probably a whole bunch of convenient helpers to help {N} developers built on top of native apis therefore I'd suggest something like this:

import { nativeExtend } from 'tns-core-modules/utils/utils';

// class implementations...

nativeExtend(Delegate, UIResponder);

If there were a large enough amount of helpers just for native api targeted work I'd suggest a new barrel just for those specific helpers even.

Ideally the entire tns-core-modules namespace could be renamed to @nativescript and could have something like:

import { nativeExtend } from '@nativescript/utils';

// class implementations...

nativeExtend(Delegate, UIResponder);

/cc @bradmartin thoughts ^

@bradmartin

This comment has been minimized.

Copy link

@bradmartin bradmartin commented Nov 19, 2018

I definitely lean toward the naming @NathanWalker mentions. I'd never know or think to look for __ns_extend_native on global. JS developers have heard for years to not pollute global. So having it be global, to some people, paints a bad image in their head. That may not be the best approach for developers of all skill levels. Having something in the core-modules, however, I'd expect.

Also the imports off of @nativescript has been something I've wanted and hoped for years. I've never liked import tns-core-modules for the reasons: its more troublesome to type by hand with the hyphens and you have to know what tns whereas nativescript is crystal clear what you are importing from. Similar to how other frameworks and companies publish packages.

Off topic question slightly, does webpack remove anything from global if not used?

@mbektchiev

This comment has been minimized.

Copy link
Collaborator

@mbektchiev mbektchiev commented Nov 20, 2018

I agree with the concerns for global pollution, but currently by design every single C function, Objective-C class, protocol, etc. do pollute it as they are attached as its properties. As such a new function, will have to be implemented in Objective-C it will also have to be a property of global.

Having said that I really like the idea to provide a more JS-friendly wrapper in the modules for the new function and not force developers to write the whatever-cumbursome-underscored-function-name we come up with.

I think it would be a major breaking change to remove all of these properties from global and it is currently not being considered. Likewise, I think that renaming tns-core-modules will definitely be another breaking change that we should carefully think over before deciding whether it's worthy to do.

@bradmartin

This comment has been minimized.

Copy link

@bradmartin bradmartin commented Nov 20, 2018

Ahhh good to know about the existing C functions being on global. If that is consistent then it makes sense to stay consistent. Having those methods exposed in core-modules should provide a few benefits: no need to create a declaration that defines all the global methods available, more JS friendly for development workflow. Of course, the exported methods from core-modules can just call the global method it's aligned with. I'm not sure how "testable" that is but I imagine it's possible.

As for the having tns-core-modules be exported under @nativescript, I haven't done any research but I'm curious if it's possible to have some alias for packages so that either would be valid going forward so it's not a breaking change but rather an enhancement, if that makes sense 😄 . It might not be possible to do without getting hacky, but I'm not sure on this area. @NathanWalker has definitely spent more time thinking about this.

@NathanWalker

This comment has been minimized.

Copy link

@NathanWalker NathanWalker commented Nov 21, 2018

Makes sense @mbektchiev - I was under impression that __ns_extend_native would be just a JavaScript method that would extend as suggested in this comment you provided ((UIResponder as any).extend({ ...etc. which works great actually.

If the method is just JS makes sense of course as an exported method from {N} framework barrels and not on global scope. I like native classes part of global scope with {N} actually, never bothered me.

However perhaps I was missing something with your suggestion of __ns_extend_native - is there a reason that would need to be a native method as opposed to a JavaScript helper which would simply use .extend as you had suggested and which works great?

@bigopon

This comment has been minimized.

Copy link

@bigopon bigopon commented Nov 24, 2018

I was extending Array and realized NS always wanted to target es5, then look up on GG to see if ES6 is supported, then led here. Would be nice if the team could provide some context to point out the limitation of the platform and how it connects to this incapability of supporting ES6. Atm, it feels much like a black box: "no you don't do it, that's all".

@manijak

This comment has been minimized.

Copy link

@manijak manijak commented Mar 18, 2019

I have been going nuts for 3 days trying to figure out why the Protocol based class in my TypeScript plugin would not work. While the vanilla js version did work. And just by coincidence I stumbled upon this issue here.

After applying the workaround mentioned by @mbektchiev it finally worked! This should be posted on the documentation for subclassing objc classes. The example written there does not work: https://docs.nativescript.org/core-concepts/ios-runtime/how-to/ObjC-Subclassing#typescript-support

@mbektchiev

This comment has been minimized.

Copy link
Collaborator

@mbektchiev mbektchiev commented Mar 20, 2019

Hi @manijak, thank you for bringing our attention to this inconsistency in the documentation and sorry for the inconvenience this has caused you. We'll update the article with a short notice about the problem and a link to this issue.

@adrian-niculescu

This comment has been minimized.

Copy link

@adrian-niculescu adrian-niculescu commented Mar 21, 2019

Is ES6 going to actually be addressed? Using ES6 is no longer optional, but essential in 2019! The workaround to use TypeScript and compile it to ES5:

  • is detrimental to performance.
  • is very bad for debugging. Probably the source mapping TS <-> JS is hard and thus buggy, but TypeScript code becomes hard to debug very fast because of this. Setting the target to ES6 improves the debugging experience in Visual Studio Code.
  • makes for a slower build.
@mbektchiev

This comment has been minimized.

Copy link
Collaborator

@mbektchiev mbektchiev commented Mar 21, 2019

It actually should be possible to switch to ES6 support now with the only drawback that you'll have to do this trick whenever you need to extend a native class in your TS code.

Other than that, we may be able to introduce the __ns_extend_native helper function which I suggested, but it still hasn't been planned for implementation by our team. If we see growing demand for this feature we will definitely raise its priority. But if the alternative method is working as expected, I don't see an immediate need for such a new feature in the runtime. Another solution could be to provide a helper function in the tns-core-modules, which would encapsulate the ugly cast to any.

@adrian-niculescu

This comment has been minimized.

Copy link

@adrian-niculescu adrian-niculescu commented Mar 21, 2019

@mbektchiev Thank you for your response. This here is not a major issue and I can see why it's not a priority, but I know there are other problems that delay the support for ES6. It's a shame given that you created your premises - both JSCore and your V8 version support much newer features, so it's not the core runtime that is the blocker anymore.

For example, we are required to deliver ES6 code, and I can't simply use ES6 JavaScript modules inside NativeScript without having to transpile them with tsc, which complicates the build process and increases the development time.

@NathanaelA

This comment has been minimized.

Copy link

@NathanaelA NathanaelA commented Apr 8, 2019

@mbektchiev - What about a feature request to actually update the extends keyword in JavaScript core engine to have the magic sauce that __ns_extend_native does. This way you depreciate the silly __ns_extend_native and the code starts magically working and this issue disappears as using extends is very common now.

@farfromrefug

This comment has been minimized.

Copy link

@farfromrefug farfromrefug commented Apr 10, 2019

actually would love to see that one too. For me this important to have es2015 target in my tsconfig. right now.you need es5 to get __extends for native inheritance. but it means you loose native async/await and other features.
what about an optional decorator? I think it should be doable.

@Gamadril

This comment has been minimized.

Copy link

@Gamadril Gamadril commented May 6, 2019

It actually should be possible to switch to ES6 support now with the only drawback that you'll have to do this trick whenever you need to extend a native class in your TS code.

I work with JS Code only and need to extend NSObject for implementing own UITextFieldDelegate.

ES6 approach: class UITextFieldDelegateImpl extends NSObject does not work. VM is complaining about UITextFieldDelegateImpl not being a native object (Exception with thrown value: Error: This value is not a native object.)

How to apply the hack for TS transpiler above to pure JS code?

@NathanaelA

This comment has been minimized.

Copy link

@NathanaelA NathanaelA commented May 6, 2019

@Gamadril - The link you quoted which links to the post; the very first example...

const Del = (UIResponder as any).extend({
    applicationDidBecomeActive(application: UIApplication): void {
        console.log("applicationDidBecomeActive", application);
    }
}, {
    protocols: [UIApplicationDelegate]
});

You have to currently use .extend() , you cannot write it as normal TS class extends NSObject

@Gamadril

This comment has been minimized.

Copy link

@Gamadril Gamadril commented May 6, 2019

@NathanaelA: thanks, got it. TS notation confused me...

Got it working that way in ES6:

const UITextFieldDelegateImpl = NSObject.extend({
   ...
}, {
    protocols: [UITextFieldDelegate]
});

UITextFieldDelegateImpl.initWithOwner = function(owner) {
    const delegate = UITextFieldDelegateImpl.new();
    delegate._owner = owner;
    return delegate;
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.