cannonicalizing url so that it can find the matching script when resolving breakpoint#379
Conversation
… scenario (otherwise, the breakpoint moves)
|
I can see the upstream issue but that issue tracker isn't public, could you copy it into this repo? Also, if you debugged this could you show why the url needs to be canonicalized here? Is this another file: URI case? |
|
Hi, I'm sorry for doing this so late, but yes, I've added the issue I to this repo: #380 Yes, it's another file:URI case. The problem shows up in OnBreakpointsResolved, when it is looking for a matching breakpoint for breakpoints in routing files in commited breakpoints. The filepath in committed breakpoints in routing files is canonicalized (though the main app.js file is not canonicalized...) But the url it is searching for has the file:/// prefix:
I am going to make another commit because although in the instances I've seen so far, only breakpoints in routing files are searched for in OnBreakpointResolved (likely due to lazy loading, although I'm not 100% sure), so the fact that the main app.js file is not canonicalized has not caused any issues in my own testing. However, I'm not sure if that would come up as a problem anywhere else, so I am planning to change it so that it checks in the committed breakpoints for the given file path, and then checks for the canonicalized filepath if it cannot find the given one. Would that be a better approach? |
| let canonicalizedUrl = utils.canonicalizeUrl(script.url); | ||
| committedBps = this._committedBreakpointsByUrl.get(canonicalizedUrl); | ||
| if (committedBps) { | ||
| script.url = canonicalizedUrl; |
There was a problem hiding this comment.
I'm not sure about mutating something that was already saved in scriptsById. If it should be canonicalized at this point, then that should be done at the point it was saved into scriptsById
|
Thanks for the details! |
| const committedBps = this._committedBreakpointsByUrl.get(script.url) || []; | ||
| // If we cannot find the script url in the committed breakpoints, we check for the canonicalized path. If found, use those committed breakpoints | ||
| // If we cannot find either the given url or the canonicalized url (or they are the same and the script simply cannot be found), committed breakpoints is an empty array | ||
| let committedBps = this._committedBreakpointsByUrl.get(script.url); |
There was a problem hiding this comment.
If the scripts inside are canonicalized we should be able to just search for the canonicalized version
|
|
||
| const committedBps = this._committedBreakpointsByUrl.get(script.url) || []; | ||
| // committed breakpoints (this._committedBreakpointsByUrl) should always have url keys in canonicalized form | ||
| const committedBps = this.getValueFromCommittedBreakpointsByUrl(script.url); |
There was a problem hiding this comment.
What will happen if there is no commitedBps for this url?
There was a problem hiding this comment.
Sorry I forgot to put back the empty array
…ebug-core into handleFilePrefixInAttach
the breakpoint is moving b/c it cannot match authored to generated path in node scenario: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/685518