fix(ng-update): properly handle update from different working directory #19933
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In some situations, developers will run `ng update` from a sub directory of the project. This currently results in a noop migration as file system paths were computed incorrectly. This is because ng-update always assumed that the CWD is the workspace root in the real file system. This is not the case and therefore threw off path resolution. We can fix this by determining the workspace file system path from the virtual file system host tree's. This is not ideal, but best solution for now until we can parse/resolve TypeScript projects purely from within the virtual file system. This is currently not easy to implement and requires helpers from TS which are not exposed yet. See: microsoft/TypeScript#13793. This is tracked with: COMP-387. Fixes angular#19779.
devversion
added
P2
googlebot
added
the
cla: yes
jelbourn
reviewed
Jul 9, 2020
| if (!(tree.root instanceof HostDirEntry)) { | ||
| return null; | ||
| } | ||
| const hostTree = tree.root['_tree']; |
There was a problem hiding this comment.
Add a comment about this private accesses in this function?
| * Gets the workspace file system path from the given tree. Returns | ||
| * `null` if the path could not be determined. | ||
| */ | ||
| // TODO: Remove this once we have a fully virtual TypeScript compiler host: COMP-387 |
There was a problem hiding this comment.
nit: the TODO should go above the JsDoc block
|
Thanks for reviewing. I went ahead and made the long-term changes that we need anyway in framework too, rather than fiddling more around with the non-ideal tricks to get the TS compiler API work for schematics. See: #19934 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some situations, developers will run
ng updatefrom a sub directoryof the project. This currently results in a noop migration as file
system paths were computed incorrectly. This is because ng-update
always assumed that the CWD is the workspace root in the real file
system. This is not the case and therefore threw off path resolution.
We can fix this by determining the workspace file system path from
the virtual file system host tree's. This is not ideal, but best
solution for now until we can parse/resolve TypeScript projects
purely from within the virtual file system. This is currently
not easy to implement and requires helpers from TS which are
not exposed yet. See: microsoft/TypeScript#13793.
This is tracked with: COMP-387.
Fixes #19779.