fix(ngcc): capture path-mapped entry-points that start with same string #35592
Conversation
petebacondarwin
added
type: bug/fix
action: review
| return true; | ||
| // Use `relative()` rather than `startsWith()` to avoid false positives for paths like | ||
| // `abc/defg` and `abc/def` since the former is not contained by the latter. | ||
| return index === array.length - 1 || relative(value, array[index + 1]).startsWith('../'); |
There was a problem hiding this comment.
-
AFAICT,
relative('/foo/bar', '/foo')will return..(without a trailing/) and thus the.startsWith('../')check will fail.
(Although, I may have missed something, because I would expect a test to be failing due to this 😕) -
By not checking against all following paths, we will incorrectly retain
a/b/din['a/b/d', 'a/b/c', 'a/b'].
There was a problem hiding this comment.
😱
That'll teach me for holiday coding
😇
I'll fix that ASAP
Previously if there were two path-mapped libraries that are in different directories but the path of one started with same string as the path of the other, we would incorrectly return the shorter path - e.g. `dist/my-lib` and `dist/my-lib-second`. This was because the list of `basePaths` was searched in ascending alphabetic order and we were using `startsWith()` to match the path. Now the `basePaths` are searched in reverse alphabetic order so the longer path will be matched correctly. // FW-1873 Fixes angular#35536
petebacondarwin
force-pushed
the
ngcc-fw-1873-path-mapped-libs
branch
from
4a33281
to
aa48cfc
Compare
|
@gkalpak - fixed the code and tightened up the test. PTAL |
| * For example: | ||
| * Given `['a/b/c', 'a/b/x', 'a/b', 'd/e', 'd/f']` we will end up with `['a/b', 'd/e', 'd/f]`. | ||
| * (Note that we do not get `d` even though `d/e` and `d/f` share a base directory, since `d` is not | ||
| * one of the base paths.) |
petebacondarwin
added
action: merge
…ng (#35592) Previously if there were two path-mapped libraries that are in different directories but the path of one started with same string as the path of the other, we would incorrectly return the shorter path - e.g. `dist/my-lib` and `dist/my-lib-second`. This was because the list of `basePaths` was searched in ascending alphabetic order and we were using `startsWith()` to match the path. Now the `basePaths` are searched in reverse alphabetic order so the longer path will be matched correctly. // FW-1873 Fixes #35536 PR Close #35592
|
@petebacondarwin Is this already in 9.0.6. ? |
|
I can confirm this does not fix ##35536, as I need to run |
|
It was included in 9.0.3 - https://github.com/angular/angular/blob/master/CHANGELOG.md#903-2020-02-27 @fkolar Please provide a reproduction of your problem in a new issue for us to investigate. |
|
Let me comit my changes.. I will create new issues. thanks allot. |
even I need to run rm -Rf dist && ng build rules && ng build material-rules && npx ngcc && ngcc -s dist otherwise there is compilation error for the application: angular/angular#35592
even I need to run rm -Rf dist && ng build rules && ng build material-rules && npx ngcc && ngcc -s dist otherwise there is compilation error for the application: angular/angular#35592
|
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. |
Previously if there were two path-mapped libraries that are in
different directories but the path of one started with same string
as the path of the other, we would incorrectly return the shorter
path - e.g.
dist/my-libanddist/my-lib-second. This was becausethe list of
basePathswas searched in ascending alphabetic order andwe were using
startsWith()to match the path.Now the
basePathsare searched in reverse alphabetic order so thelonger path will be matched correctly.
// FW-1873
Fixes #35536