Skip to content
This repository was archived by the owner on Aug 7, 2021. It is now read-only.

Bundler causes serious weird/crashing issues with ios ng apps with several plugins. #8

Closed
NathanaelA opened this issue Jul 13, 2016 · 22 comments

Comments

@NathanaelA
Copy link

NathanaelA commented Jul 13, 2016

I have traced down a couple ios issues caused by webpack when running with multiple plugins:

  1. __extends will run on NSObject code even though, I believe you hijack it normally inside the
    https://github.com/NativeScript/ios-runtime/blob/b7b8d69c1857d6d6293b1e33c60ac6980a4161e3/src/NativeScript/ObjC/Inheritance/ObjCTypeScriptExtend.mm#L49 -- I don't know if this is an issue for sure; but in the webpack'd code -- when this runs, it attempts to run the __() which set the this.constructor (this.constructor = d;) which when the code is set to "use strict;" will cause it to crash. This is NOT an issue if the file isn't web packed. (In fact in my tests, the normal JS __extends --- __() constructor code does not appear to be ran against a NSObject at all); however in a webpacked version the code is ran which is what causes the failure.
  2. the Constructor itself https://github.com/NativeScript/ios-runtime/blob/b7b8d69c1857d6d6293b1e33c60ac6980a4161e3/src/NativeScript/ObjC/Inheritance/ObjCTypeScriptExtend.mm#L34 is supposed to be hijacked by this regex; but the regex breaks on webpacked code, and so the _super.apply is ran. A NSObject based object in NS 2.1 doesn't have a .apply() function; which will cause the app to crash when it attempts to run it.
  3. WebPack can't seem to understand the radlistview listview from ng2 code, it can't seem to resolve the file/folder properly. (Renaming listview.ios.js to listview.js inside the rad listview folder allows it to works)
  4. Using a NPM link on the nativescript-angular folder will cause serious webpack pathing issues -- it appears to be trying to go up to the parent folder of where the final symlink is at; rather than the parent folder inside the node_modules (finally manually just deleted the link, and copied the rc4 version folder manually into the node_modules folder)
@jasssonpet
Copy link

@NathanaelA

Regarding the first two issues, are you using the UglifyJS plugin or is this behavior so in the default config.

@NathanaelA
Copy link
Author

I just do a
tns install webpack
then
tns run ios --bundle --emulator

As far as I know no uglifyJS is being activated;

The webpack config file:

var bundler = require("nativescript-dev-webpack");

module.exports = bundler.getConfig({
    // TODO: add project-specific webpack settings here...
});

@NathanWalker
Copy link
Contributor

I am currently blocked by this issue in a bad way :( I have an app I would like to deploy to app store but cannot bundle it properly due to these issues. The unbundled app is 136 Mb so it would be unacceptable to post such a large app which I want to be a great production example of a nativescript-angular app.

@jasssonpet
Copy link

jasssonpet commented Jul 14, 2016

Does the first iOS extends issue happen with a blank application (tns create MyApp --ng)? I've tried with both a blank app and the ng-sapmle and can't seem to reproduce it.

If you could send a sample or just the extend snippet that's happening I'll be more than interested to investigate it further.

@NathanaelA
Copy link
Author

Notice the title, with plugins. :-) That is probably the missing factor.

I have figured a way around all 4 issues (all though all 4 issues are still valid issues). They are no longer blocking for me (or Walker). The first two issues are causes by several of the plugins having their own __extends function. This version of the __extends is then inlined by webpack and then space/tab padded which makes it change the "fingerprint" of the __extends function causing it to fail the built in ios runtime __extends fingerprint regex check.

My solution was to add a webpack plugin that strips all __extends from the source as it packs the bundle. This seems to be the simplest solution, and it appears that it worked for me, as I was able to get quite a ways into the app before it crashed again. (See #10 )

This issue will probably hit anyone using any number of popular plugins as a large chunk of the plugins that ship have the built in __extends function in final js file. And when webpack bundles them up; it will crash with all sorts of interesting fireworks. :-)

@jasssonpet
Copy link

Can you give an example of such plugin, I want to try it out.

@NathanaelA
Copy link
Author

nativescript-cardview, nativescript-ezaudio, nativescript-gif, nativescript-intl, nativescript-spotify
are some of them that have their own __extends. Please note, NS-spotify is the one that I was seeing the issue; but that might be just because ns-spotify was the last of the plugins to load that had an __extend (so it overwrites the global one)...

@jasssonpet
Copy link

It shouldn't rewrite the global __extends function, because it's marked readonly but I'll take a look.

@NathanaelA
Copy link
Author

Well, let me restate it; I think that this is the FIRST one being defined; because it was where the crash was always occurring. The line number was always in the ns-spotify/src/ios/notification.js __extends, in side the __() { this.constructor = d; }

It took me a little while to figure out what was causing the error with TypeError read only property being set. It ended up being the actual NSObject from the TNSSpotifyNotificationObserver in this file.

Once I eliminated the "use strict"; at the top; it was fine because then it would "ignore" the read only type error on the attempt to re-assign; then I got the error with the _super.apply constructor because it was being fired on a NSObject (and NSObject's don't apparently have an "apply" function).

@leocaseiro
Copy link

leocaseiro commented Jul 18, 2016

I have an issue which is might be related to this.

I'm using Angular and every time that I try to use registerElement, it seems not working.

I'm new to webpack still, but I can tell only when I compile with --bundle I'm getting the issue:

ORIGINAL EXCEPTION: TypeError: Could not load view for: PullToRefresh.TypeError: Attempted to assign to readonly property.

The plugin I'm testing now is nativescript-pulltorefresh.

registerElement("PullToRefresh", () => require("nativescript-pulltorefresh").PullToRefresh);

I've tried using globals and global.require as well, but doesn't give me bugs, as well as not working.

UPDATE
I removed strict mode as well, so now I'm getting the error:

Error: This value is not a native object.

@leocaseiro
Copy link

Thanks @NathanaelA, I fixed it removing the __extend function and removing the 'use strict' as well, so I believe if we request to all {N} plugin developers to remove the __extends() function, it'll work fine:

http://stackoverflow.com/questions/36556772/webpack-and-typescript-extends

Perhaps @NathanWalker would like to add this *.ts setup to the nativescript-ng2-plugin-seed

leocaseiro added a commit to leocaseiro/nativescript-ng2-plugin-seed that referenced this issue Jul 19, 2016
TypeScript creates an `__extends` function which [breaks most of apps in {N}](NativeScript/nativescript-dev-webpack#8).

A simple fix is settings to `noEmitHelpers` to true.
leocaseiro added a commit to leocaseiro/nativescript-pulltorefresh that referenced this issue Jul 19, 2016
Adding noEmitHelpers to TypeScript avoid the creating of the __extends() function which breaks WebPack.

Read more: 
NativeScript/nativescript-dev-webpack#8
http://stackoverflow.com/questions/36556772/webpack-and-typescript-extends
leocaseiro added a commit to leocaseiro/nativescript-google-maps-sdk that referenced this issue Jul 19, 2016
Adding `noEmitHelpers` to TypeScript avoid the creating of the `__extends()` function which breaks WebPack.

Read more: 
NativeScript/nativescript-dev-webpack#8
http://stackoverflow.com/questions/36556772/webpack-and-typescript-extends
@NathanaelA
Copy link
Author

@leocaseiro I actually wrote a webpack plugin automatically filters out all the __extends, right after I posted this issue. Very very few things actually "stop" me for any amount of time. ;-)

I would leave the "use strict" this actually allows the v8 engine and JSC engine to do certain optimizations; eliminating "use strict" makes the engine be in compatibility mode...

@leocaseiro
Copy link

Hi @NathanaelA, would you mind sharing this webpack plugin?

@NathanaelA
Copy link
Author

@leocaseiro When I have some time, I'll post it somewhere. Trying to get a couple clients projects done... You might need to ping me later this week as I might forget all about this...

@NathanaelA
Copy link
Author

@leocaseiro I posted it in a blog post here:
http://fluentreports.com/blog/?p=342

@leocaseiro
Copy link

Many, many thanks @NathanaelA.

You are a legend! So far!

Perhaps, you'd like to add to npm?

@NathanaelA
Copy link
Author

@leocaseiro Not a problem, glad to help you out.

It actually would be trivial to turn it into a plugin that even self-installs; but to be honest; I don't have the "free" time now. I've had to make some hard choices recently, and so my NS free time has shrunk drastically. So drastically I don't bother answering questions on StackOverflow anymore, and I really enjoy helping people. It has even taken me a couple weeks to actually fix my own LiveEdit plugin to finally work with the latest runtimes (and I use this plugin myself in all my projects). To be honest I'm not even sure if I will find the time to even release even those fixes to the community any time soon. The bills have to be paid from chasing contracts which are almost always outside of the NS community, so now that is where I have to focus... So now everything NS related pretty much stays locally, as it takes way too much "free" time to package things up. What really sucks is beyond the already large collection of unreleased plugins that I'm using in my own projects, is I still have so many other great ideas on how to enhance the NS community. Sadly due to a lack of funding I have finally faced the reality that very few of them will ever come to pass -- ah well, life goes on... Maybe I'll find some company that will want something more than a couple weeks work here and there, and so I'll get more free time, but as it stands now...

@leocaseiro
Copy link

@NathanaelA I understand that. We all need to prioritise work that pay bills.

If you authorise me, I'd be happy to release an npm module and, of course, I'd add you as a collaborator.

Let me know your thoughts.

For the moment, I have this method running locally anyway.

Again! Many many thanks for your big help and time.

@hdeshev
Copy link
Contributor

hdeshev commented Jul 28, 2016

Hi guys,

Sorry for replying so late, but I wanted to check all the cases @NathanaelA listed above.

To me, the biggest problem is # 1 since it relies on plugin writers getting that noEmitHelpers option right. I hit the problem with a couple of plugins, but those were authored by Telerik guys, and we fixed them quickly. As for the rest of the plugins, we'll either have to start submitting PR's, but in the meantime the loader solution seems to be the better option. We should really release it on npm.

I wasn't able to reproduce issues 2 and 3. I got the same _super.apply code in my bundles as in the nonbundled code, and the iOS runtime magic regex seems to get it covered. I also got our marketplace demo bundled for iOS, and its radlistview examples seem to work. Is it possible that you were using some webpack setting, like uglification plugin? I remember seeing some problems with minified code in the past and since we got no real benefit from minification performance-wise, I gave up investigating those in depth.

Issue 4 seems like some corner case in webpack. I managed to reproduce it, and got to the point of the enhanced-resolve ResultSymlinkPlugin resolving that file to an incorrect path. I don't know what to do about it, given it seems a rare case. I'm not 100% sure our custom resolver plugin isn't to blame yet since I couldn't reproduce the issue with a simple linked commonjs module. I'll just log it as a separate issue.

@NathanaelA
Copy link
Author

NathanaelA commented Jul 28, 2016

@hdeshev -
\1. Yes, you can do whatever is needed to get it fixed including bundling my plugin with the ns-dev-webpack or releasing it as standalone (I would think including it with the ns-dev-webpack plugin would be better, because then when you create the webpack config file, you can just auto point to run out of the node_modules/ns-dev-webpack/plugin.name.js

\2. On issue number 2 this is directly related to first issue, if some of the extra __extends are present then the regex fails and so then they are ran and then crashes occur. I don't have any ideas WHY the regex failed; the __extends looks correct and looks like it should match the regex. I actually stepped through this code in the debugger many times trying to understand what was going on; that was fun... :-D If you need a failing case I'm pretty sure @NathanWalker can give you access to the repo and then you just need to delete the webpack config to eliminate the use of my extend removing plugin and you will see it crash during startup. If you remove the "use strict" in that specific version of the __extends; you will easily see number 2 occur, because then the __extends actually runs fully (w/o failing from a strict violation) and sets up the NSObject improperly and then the .apply() is attempt when the class is actually new'ed. -- Thinking about this; I wonder if you never see number 2, because the versions of the __extends you have the "use strict" set which would always cause the failure to stop the code as issue number one.

When I reported this issue; my webpack config file was totally empty; I did NOT apply any minification or anything else. In fact this project for Nathan was my first dive into Webpack; I was completely unaware of the webpack config file until I needed to figure out some way to fix the issue and then I wrote the plugin and added it to the empty wepback config. So rest assured, this was a completely stock ns-dev-webpack install. :-)

\3. This might be fixed now, the ng2 bug I saw yesterday was also reported by someone else in the Telerik UI Forums, and there has been a release or two of the NS-UI since that point, so @ginev might have already fixed this issue...

\4. Yeah, that is a corner case -- I don't know if it needs to be fixed or just documented. It was a surprise that occurred while dealing with the other issues that I ran into.

@leocaseiro
Copy link

I've just had the same issue with RadListView for Android as well.

I had to rename from listview.android.js to listview.js.

I'm using Angular, and it's only happens after include the Provider:

import {LISTVIEW_PROVIDERS} from 'nativescript-telerik-ui-pro/listview/angular';
nativeScriptBootstrap(myApp, [LISTVIEW_PROVIDERS]);

@hdeshev
Copy link
Contributor

hdeshev commented Dec 5, 2016

Closing this one, since we have separate issues for the problems. The latest release of RadListView should be bundlable.

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

No branches or pull requests

5 participants