Skip to content

Conversation

hdeshev
Copy link
Contributor

@hdeshev hdeshev commented Apr 15, 2016

Webpack bundling will split the final bundle in two chunks:

  • tns-java-classes.js: containing NativeScriptApplication, NativeScriptActivity, and all related code.
  • bundle.js: containing the rest of the library and application code.

We need to load tns-java-classes (if present) earlier, so that Java-JavaScript mappings get registered.

Here's a sample tns-java-classes.js before webpacking -- those require calls will get changed by webpack.

Ping @jasssonpet, @KristinaKoeva

@blagoev
Copy link
Contributor

blagoev commented Apr 15, 2016

looks ok.
Can we do this using Javascript only. Maybe changing the ts_helpers.js file to require the app/tns-java-classes.js if it exists?

Or maybe we can change the runtime to run every module in internal dir on startup and place the tns-java-classes.js there.

@ns-bot
Copy link

ns-bot commented Apr 17, 2016

💚

@hdeshev
Copy link
Contributor Author

hdeshev commented Apr 19, 2016

@blagoev I moved the tns-java-classes.js load code to ts_helpers.js, but that feels worse than the original setup:

  • I had to change RuntimeHelper to use runModule instead of runScript for ts_helpers.js
  • The ts_helpers.js code doesn't have app dir information, and I had to take it from __dirname (the reason I switched to runModule above.
  • The code is a bit more complex.

I suggest I drop the last commit and merge the Java (RuntimeHelper) fix I initially proposed.

@ns-bot
Copy link

ns-bot commented Apr 19, 2016

💚

@jasssonpet
Copy link
Contributor

jasssonpet commented Apr 22, 2016

👍 Something like this would be required for the snapshot plugin, too - NativeScript/NativeScript#1563

@atanasovg
Copy link
Contributor

I think the initial proposal looks better. Ideally, we would do what @blagoev suggests - to traverse the internal directory and run every JS file found there. But given the short time frame before the release I suggest we just merge @hdeshev initial proposal - 👍 from me for this.

@hdeshev hdeshev force-pushed the hdeshev/webpack-fix branch from f276f83 to f858e75 Compare April 22, 2016 08:45
@hdeshev
Copy link
Contributor Author

hdeshev commented Apr 22, 2016

Reverted to the initial implementation. Merge at will. 👼

@ns-bot
Copy link

ns-bot commented Apr 22, 2016

💚

@blagoev
Copy link
Contributor

blagoev commented Apr 22, 2016

👍 for the traverse of internal dir we may need ordering, so lets just keep this for now

@atanasovg atanasovg merged commit 1d960bb into master Apr 22, 2016
@jasssonpet jasssonpet deleted the hdeshev/webpack-fix branch April 27, 2016 11:09
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.

5 participants