Normalize tsconfig path #73001
Normalize tsconfig path #73001
Conversation
|
|
| } | ||
| return project.path; | ||
| return normalizePath(projectPath); |
There was a problem hiding this comment.
@alexr00, done as suggested. Had to use a custom normalizePath() since path.posix.normalize() does not convert win32 paths to unix.
There was a problem hiding this comment.
One thing to note as you mentioned earlier, all the tasks.json files around the world which have windows path separators will show 'There are task errors' dialog with the below output:
There was a problem hiding this comment.
That is correct. We did something similar with ${relativeFile} here
There was a problem hiding this comment.
@alexr00, If all is good, can you please approve this PR.
There was a problem hiding this comment.
Got it. I was just hoping for an approval of the PR, not merge. Non the less, I now understand that the PR is accepted.
PS: This 'we are in the endgame now' gives me chills!
|
@alexr00 and @NizamLZ, not sure of the best way to get some clarification about this change... I was just auto-updated to the latest version of VS code and noticed that my custom tasks now always caused an error: After quite a bit of time spelunking the VS Code update list and following many issue links, I discovered that this change appears to have broken all existing and working tasks.json of this form. This is particularly notable because, from what I can tell, the newly required form (forward slashes instead of backslashes) does not work on older versions. It seems like the ramifications of this are that literally all VS Code users who has custom tasks of the old form will immediately break on upgrade (which they may not even realize happened, since it frequently happens automatically). Moreover, it will not be obvious to them what broke or how to fix it: there's no obvious error message or indication of exactly the new requirement. When I did come across this thread, I was surprised to find that this exact issue was called out above: we're going to break all Windows users and seemingly dismissed. While I don't want to suggest that breaking changes can never be introduced, it seems like this particular change is going to cause untold developer hours trying to understand why their tasks broke suddenly, how to fix the problem, etc. Is it possible to elaborate on the threshold for when breaking changes like this can be introduced without any warning or obvious remediation alerts? Are there times when backwards compatibility should be considered? Or maybe I'm misunderstanding the impact (i.e. will there be fewer devs broken by this than I think and somehow I got lucky?) Thanks! |
|
@erinha you are right that there should have been some sort of announcement for this. I should have included it in the release notes. In this case, the breaking change fixed a bug (typescript tasks could not be customized in a cross platform way). We have a precedence for this kind of change (#70050). This change will impact Windows users who have customized typescript tasks. |
Normalize tsconfig path before task identifier generation to resolve #68812
Cause:
The task identifier is generated using the required fields of a given task type. For typescript, the value of the
tsconfigis included in the task identifier. Typescript extension is generating the value of tsconfig using the platform specific path separator(irrespective of the value in tasks.json) but thecreateTaskIdentifier()in common/tasks.ts is using the value present in tasks.json.Hence
configuringTaskin task.contribution.ts#L1467 is evaluated asnull(the task identifiers don't match), making the task invalid.This PR normalizes the value of tsconfig before the task identifier is generated.