Skip to content

Invalid XML kills livesync #1777

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

Closed
bradmartin opened this issue May 25, 2016 · 6 comments
Closed

Invalid XML kills livesync #1777

bradmartin opened this issue May 25, 2016 · 6 comments
Assignees
Labels
Milestone

Comments

@bradmartin
Copy link

Using livesync, if you bork the XML and save changes. The app crashes, which is fine, but then after correcting the invalid XML you have to do a new build for Android to get the app to livesync again. Big time killer. It's come up on the slack channel a couple of time and confuses people who don't know how to get around it.

@enchev
Copy link
Contributor

enchev commented May 26, 2016

Hey @bradmartin,

Maybe we should stop deploying broken XML at all? Right now you will get only warning from the CLI if the XML is invalid however we can change that to real error which will stop the current task (livesync, run, etc.).

Hey @atanasovg @dtopuzov @hdeshev @hshristov @PanayotCankov @rosen-vladimirov what do you think?

@bradmartin
Copy link
Author

Anything to prevent the need to rebuild and start livesync over I'm all
for. Sometimes I even forget to do that and get frustrated and I've been
around for awhile so I just don't want newer NativeScript devs to get
frustrated and give up 😄 I trust you guys to implement a good
solution. Most of the time for me my IDE might throw in an extra double
quote when im making quick attribute property changes. I know I can change
the IDE auto completion settings but that's besides the issue. So again
thanks for getting a discussion going so this can be resolved. Its my only
livesync complaint to be honest.

On Thu, May 26, 2016, 8:32 AM Vladimir Enchev [email protected]
wrote:

Hey @bradmartin https://github.com/bradmartin,

Maybe we should stop deploying broken XML at all? Right now you will get
only warning from the CLI if the XML is invalid however we can change that
to real error which will stop the current task (livesync, run, etc.).

Hey @atanasovg https://github.com/atanasovg @dtopuzov
https://github.com/dtopuzov @hdeshev https://github.com/hdeshev
@hshristov https://github.com/hshristov @PanayotCankov
https://github.com/PanayotCankov @rosen-vladimirov
https://github.com/rosen-vladimirov what do you think?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1777 (comment)

@rosen-vladimirov
Copy link
Contributor

Hi guys,
I believe we should not stop the livesync when there's invalid XML, but we should not livesync the file as well. My idea is to validate the file and tell the user: "Hey, this file's incorrect, fix it, save it and we'll do the job for you". So the user will not restart the operation, we'll check the file and if we consider it as valid, we'll send it to device.

@enchev
Copy link
Contributor

enchev commented May 27, 2016

We have similar issue for TypeScript compilation. In both cases (XML is not well formed and TypeScript cannot be compiled) in my opinion we should stop the transfer (not the watch itself). When you fix all errors your files will be transferred automatically again.

@hdeshev
Copy link
Contributor

hdeshev commented May 27, 2016

I also think we should alert the user by changing the app behavior when we prevent a sync due to an error. Here's how RN does it for app crashes:

We should use that for compile errors, xml validation failures, etc.

@enchev
Copy link
Contributor

enchev commented May 27, 2016

Hey,

I've played a bit with various approaches and here is what I've found:

Approach A (Stop livesync transfer in case of XML error)

  • (pros): will notify the developer in the livesync console about errors in any XML file in the application
  • (cons): the developer may not notice the error in the console since the application will not react in any way

Approach B (Continue livesync transfer in case of XML error)

  • (pros): will notify the developer in both livesync console and the app (via alert, modal page, etc.)
  • (cons): the developer will be alerted in the app only about the current page errors. If you have invalid XML in other pages you will not know until you try to open the page at runtime.

I suggest to move on with my current pull request and research the B approach and eventually implement it in the near future.

Thanks

[UPDATE] Ok. I've managed to implement the B approach in the modules. Here is how it works:
livesync

@enchev enchev added this to the 2.1 (Under consideration) milestone May 27, 2016
@enchev enchev self-assigned this May 27, 2016
@enchev enchev closed this as completed May 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants