Skip to content
This repository has been archived by the owner on Feb 10, 2023. It is now read-only.

This plugin overrides any existing AppDelegate #24

Closed
speigg opened this issue Apr 7, 2017 · 17 comments
Closed

This plugin overrides any existing AppDelegate #24

speigg opened this issue Apr 7, 2017 · 17 comments
Assignees

Comments

@speigg
Copy link
Contributor

speigg commented Apr 7, 2017

This plugin overrides any existing app delegate:

application.ios.delegate = UrlHandlerAppDelegate;

This seems like a bad idea..

@hypery2k
Copy link
Owner

hypery2k commented Apr 7, 2017

any idea what would be a good alternative?

@speigg
Copy link
Contributor Author

speigg commented Apr 7, 2017

  1. You could try the approach that the facebook plugin uses, and not mess with the delegate at all (see https://github.com/mobilemindtec/nativescript-facebook). This is probably the simplest solution...
  2. Might be possible to subclass an existing delegate to extend it (or just override the appropriate method on the prototype directly), though I am not sure how tricky it would be to avoid conflicts with other code / plugins that might also want to mess with the application delegate. It might be safe enough in this case to just call any existing method and check if the url has already been handled or not (if it returns true or false).

@hypery2k
Copy link
Owner

hypery2k commented Apr 7, 2017

Thanks for the info. Will try to investigate on that and make a new release

@hypery2k
Copy link
Owner

so you mean just using the activity result?

@speigg
Copy link
Contributor Author

speigg commented Apr 11, 2017

Well, this issue was mainly for iOS, I don't know much about android, though it looks like they are preserving the existing callback for the activity result by wrapping. But a similar strategy would probably work well for iOS, I think (basically the same thing I described as option 2 above). I wonder why the Facebook plugin doesn't already take that approach for iOS though.

@c1ngular
Copy link

@speigg very good point

@hypery2k
Copy link
Owner

@speigg Could you submit a PR. I'm currently running out of time due to other projects

@hypery2k
Copy link
Owner

hypery2k commented Jul 4, 2017

@speigg Any suggestions so far. Though about using open directly, but it's not working.

@hettiger
Copy link

@speigg @hypery2k

I have tried to fix this. You can check out my fork: https://github.com/hettiger/nativescript-urlhandler/blob/24-plugin-overrides-existing-ios-appdelegate/src/urlhandler.ios.ts

Unfortunately this doesn't seem to work for some reason. Any ideas?

The reason for my attempt was this line:
https://github.com/EddyVerbruggen/nativescript-plugin-firebase/blob/78f60f55be30b022690722006e1080b1685548fa/src/firebase.ios.ts#L135

The plugin nativescript-plugin-firebase is used in a lot of projects and I bet other plugins do it in a similar way. I came to the conclusion that getters/setters would be a good fix but for some reason it doesn't work.

Here is a plain JS proof of concept:

function Delegate() {}

/**
 * Original / already existing handler
 */
Delegate.prototype.handler = function (first, second) {
    console.log(arguments);
    console.log('Delegate handler – Original');

    return false;
};

function enableMultipleOverridesFor(classRef, methodName) {
    const prefix = '_';

    Object.defineProperty(classRef.prototype, prefix + methodName, {
        value: classRef.prototype[methodName] || (() => {}),
        writable: true
    });

    Object.defineProperty(classRef.prototype, methodName, {
        get: function () {
            return classRef.prototype[prefix + methodName];
        },
        set: function (nextImplementation) {
            const currentImplementation = classRef.prototype[prefix + methodName];

            classRef.prototype[prefix + methodName] = function () {
                const result = currentImplementation(...Array.from(arguments));
                return nextImplementation(...Array.from(arguments), result);
            };
        }
    });
}

/**
 * It is important that this code is being executed before the first override
 * in order to guarantee all overrides will be executed.
 */
enableMultipleOverridesFor(Delegate, 'handler');

/**
 * Example for a plugin that overrides the handler
 */
(function() {
    Delegate.prototype.handler = function (data) {
        const lastArgument = arguments[arguments.length - 1];
        const previousResult = lastArgument !== data ? lastArgument : undefined;

        console.log(arguments);

        if (!previousResult) {
            console.log('Delegate handler – 1st overwrite');
            return false;
        }

        return previousResult;
    };
})();

/**
 * Example for a plugin that overrides the handler
 */
(function() {
    Delegate.prototype.handler = function (data) {
        const lastArgument = arguments[arguments.length - 1];
        const previousResult = lastArgument !== data ? lastArgument : undefined;

        console.log(arguments);

        if (!previousResult) {
            console.log('Delegate handler – 2nd overwrite');
            return true;
        }

        return previousResult;
    };
})();

/**
 * Example for a plugin that overrides the handler
 *
 * It is to be expected, that this handler will __not__ log "Delegate handler – 3rd overwrite"
 * to the console because the previously executed handler returned true.
 */
(function() {
    Delegate.prototype.handler = function (data) {
        const lastArgument = arguments[arguments.length - 1];
        const previousResult = lastArgument !== data ? lastArgument : undefined;

        console.log(arguments);

        if (!previousResult) {
            console.log('Delegate handler – 3rd overwrite');
            return false;
        }

        return previousResult;
    };
})();

const delegate = new Delegate();
const finalResult = delegate.handler('1st argument', '2nd argument');

console.log(finalResult);

You can execute this in a modern browser. It will output:

{ '0': '1st argument', '1': '2nd argument' }
Delegate handler – Original
{ '0': '1st argument', '1': '2nd argument', '2': false }
Delegate handler – 1st overwrite
{ '0': '1st argument', '1': '2nd argument', '2': false }
Delegate handler – 2nd overwrite
{ '0': '1st argument', '1': '2nd argument', '2': true }
true

@ickata
Copy link

ickata commented May 19, 2018

Hey @hypery2k , will you please let us know when the next release will be available in npm?

@ickata
Copy link

ickata commented May 19, 2018

@hypery2k ,

I just tested with a local build this fix. handleOpenURL is not invoked. iPhone 6 (iOS 11.3)

@hypery2k hypery2k self-assigned this May 19, 2018
@hypery2k hypery2k reopened this May 19, 2018
@hypery2k
Copy link
Owner

not sure if i find time for a new version

@muratcorlu
Copy link

I'm also using nativescript-plugin-firebase and I think because of this, this plugin is not working on iOS. Does anyone have a workaround or an alternative?

@Mitko-Kerezov
Copy link
Contributor

@muratcorlu I've been testing around this plugin for a while now and I think I know where your issue may reside.

Firebase directly overrides the methods it requires of the app delegate, whereas this plugin (from the develop branch) chains the methods.

In order for both plugins to work you would have to make sure that firebase is required before nativescrupt-urlhandler.

In addition my testing has lead me to believe that this chaining method does not work because properties defined with Object.defineProperty don't seem to be respected.

The actual code, which worked for me was:

function enableMultipleOverridesFor(classRef, methodName, nextImplementation) {
   const currentImplementation = classRef.prototype[methodName];
   classRef.prototype[methodName]  = function () {
       const result = currentImplementation && currentImplementation(...Array.from(arguments));
       return nextImplementation(...Array.from(arguments), result);
   };
}

Which is called like so:

enableMultipleOverridesFor(appDelegate, 'applicationDidFinishLaunchingWithOptions', () => { console.log("This will be executed FIRST") } );

enableMultipleOverridesFor(appDelegate, 'applicationDidFinishLaunchingWithOptions', () => { console.log("This will be executed SECOND") } );

I hope this helps

@hypery2k hypery2k added the bug label Mar 2, 2019
@hypery2k hypery2k closed this as completed Mar 2, 2019
@jdnichollsc
Copy link

Excuse me mates, why this issue was closed?

@hypery2k
Copy link
Owner

because the plugin itself not longer overwrites the delegate:

classRef.prototype[methodName] = function () {
const result = currentImplementation && currentImplementation.apply(currentImplementation, Array.from(arguments));
return nextImplementation.apply(nextImplementation, Array.from(arguments).concat([result]));
};

@jdnichollsc
Copy link

Ohh Interesting, do you know if is possible to do that using ES6? also, if we can create a PR from the {N} Core repo to avoid overwriting the app delegate 🤔

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants