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

fix: improve debugging of applications in third party tools and Studio #893

Merged
merged 11 commits into from
Jun 19, 2019

Conversation

feons
Copy link
Contributor

@feons feons commented Apr 5, 2018

@feons feons requested a review from hansemannn April 5, 2018 22:41
@@ -117,6 +120,8 @@ exports.generateCodeAndSourceMap = function(generator, compileConfig) {
// write the generated controller code
var outfile = target.filepath;
var relativeOutfile = path.relative(compileConfig.dir.project, outfile);
outputResult.code += `\n//# sourceMappingURL=file://${compileConfig.dir.project}/${CONST.DIR.MAP}/${relativeOutfile}.${CONST.FILE_EXT.MAP}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it is not appended to Common JS modules so far. Testing with https://github.com/hansemannn/studentenfutter-app, an advanced Alloy based app. Also, Safari does not seem to pick up the source map so far. Maybe the file:// prefix is invalid there? Testing with Android on Chrome next.

Copy link
Contributor

@hansemannn hansemannn Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feons Any update on that one? I've also requested @ewanharris for a QR as he's more into this 😙. Would be good to have source maps working for both iOS and Android, getting it out with the next feature release.

@hansemannn
Copy link
Contributor

@feons Can we finish this one?

feons and others added 3 commits May 29, 2019 10:04
This check was being performed on the destination file which never existed, we also no longer need to check for a directory here as it is checked prior to calling this function

Fixes ALOY-1690
@build
Copy link

build commented May 29, 2019

Warnings
⚠️

Please ensure to add a changelog entry for your changes. Edit the CHANGELOG.md file and add your change under the Unreleased items header

Messages
📖

✅ All tests are passing
Nice one! All 3480 tests are passing.

Generated by 🚫 dangerJS against 236413e

@hansemannn
Copy link
Contributor

@ewanharris I see activity here, whooop! :) Let me know if I can test on our larger scale project!

This ensures that the code the the sourcemap is generated lines up a best as we can get with what is handed off to the sdk build process
@nirmaljpatel
Copy link

Can one of you guide me on how this feature can be used?

For a while I was able to use Safari Developer -> Simulator -> JSContext and see my JS files in the debugger. However, this doesn't seem to be working any more atleast for me with Ti SDK8.0.1.GA.

Is there a guide somewhere elaborating how to debug with Chrome/Safari dev-tools?

@ewanharris
Copy link
Contributor

Hey @nirmaljpatel, this PR is currently going through some changes so I wouldn't recommend using it. We do have guides for debugging iOS via Safari and Android via Chrome, however be aware that for iOS there are currently some known issues TIMOB-27098 with debugging in Safari (which might be what you're referencing?)

- report absolute path for 'file' property
- report absolute path for 'sourceRoot', not a file:// URI
- embed sourcesContent for mapped files
- fix line counts in source map after hitting template marker lines
@sgtcoolguy
Copy link
Contributor

some open TODOs:

  • We write a source map for app/lib/ files externally, but we do not embed a //# sourceMappingURL= comment in the copied file to point at it. So tools like Babel or node-titanium-sdk don't "know" about the source map.
  • The whole copying of app/lib/ files is pretty hokey anyways. It copies the file, then we generate a 1:1 source map file (and in the process parse the file unnecessarily!), then after everything the "optimizing" step ends up running the babel transforms on the file when its doing it's sweep on the Resources/ folder. The "copying" portion should do the transform and generate the source map and comment all at once.
  • We should see if we can't just remove the special babel plugins/transforms altogether in Alloy (and just copy file contents with the source map stuff). I've started to try and move them to a babel plugin here: https://github.com/appcelerator/babel-plugin-transform-titanium that is intended to eventually be used by node-titanium-sdk's jsanalyze method. That way we can just have the same optimizations for all titanium builds and not have special "defines" or optimizations that only Alloy does.

Copy link
Contributor

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me. Jenkins failure is due to a node connection timeout on the master run but the GA run was fine.

@ewanharris ewanharris merged commit 2190609 into tidev:master Jun 19, 2019
@ewanharris ewanharris changed the title [ALOY-1612] Be able to use Alloy source-maps in Chrome Dev-Tools fix: improve debugging of applications in third party tools and Studio Jun 19, 2019
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.

6 participants