build(aio): add metadata aliases for directives, components and pipes #19317
Conversation
petebacondarwin
added
comp: aio
action: review
|
You can preview a1469bb at https://pr19317-a1469bb.ngbuilds.io/. |
|
Nice touch! |
petebacondarwin
force-pushed
the
aio-autolink-selectors
branch
from
a1469bb
to
a612155
Compare
|
You can preview a612155 at https://pr19317-a612155.ngbuilds.io/. |
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
I think this new line should be moved below stripQuotes() 😛
|
|
||
| function extractSelectors(selectors) { | ||
| if (selectors) { | ||
| return stripQuotes(selectors).split(',').map(selector => selector.replace(/^\W+(\w+)\W+$/, '$1')); |
There was a problem hiding this comment.
I don't know if this is intentional, but the current implementation will return selector that don't match the RegExp unchanged. E.g. foo:not(bar) will be returned as an alias and .foo will be converted to foo.
Most of the directives will not be affected as they use simple selectors, but I think it would be safer to only include aliases for selectors that match the RegExp.
Speaking of the RegExp, \w+ will not match - (it will only match letters, numbers and _ iirc).
I assume this was not intentional either? It seems useful to be able to catch selectors with - (even if there aren't (m)any atm).
For example, the current implementation will not match [my-directive].
So, I would expect something along the lines:
const re = /^\W*([\w-]+)\W*$/;
return stripQuotes(selectors).
split(',').
map(s => re.exec(s)).
filter(m => m).
map(m => m[1]);There was a problem hiding this comment.
The regex was designed intentionally.
Most of the more complex selectors require CSS parsing on both sides of the matching (i.e. here and also in the autolinker), so I didn't try too hard to make the parser here too clever. It seemed more sensible to just pass through unmatched selectors, so that they were at least available to people if they wanted to create a manual inline link e.g. {@link foo:not(bar)}.
The way that auto-linking works, it only tries to match whole words in source code snippets, so anything with a symbol in it is never going to be auto-linked anyway, even if there was an alias for it.
But I guess that since people might want to create manual links then we might benefit from a more lenient approach on the regex.
| expect(processor.$runBefore).toEqual(['computing-ids']); | ||
| }); | ||
|
|
||
| it('should add new aliases for directives and components', () => { |
| { docType: 'enum', name: 'MyEnum', aliases: ['MyEnum'] }, | ||
| { docType: 'function', name: 'myFunction', aliases: ['myFunction'] }, | ||
| { docType: 'pipe', name: 'MyPipe', aliases: ['MyPipe'], pipeOptions: { name: '\'myPipe\'' } }, | ||
| { docType: 'directive', name: 'MyDirective', aliases: ['MyDirective'], directiveOptions: { selector: '\'my-directive,[myDirective]\'' } }, |
There was a problem hiding this comment.
Missing tests for [my-directive] and double quotes 😃
There was a problem hiding this comment.
Double quotes never appear in these fields, so the strip quotes is too complicated, but I don't feel the need to test something that never occurs 🙀
There was a problem hiding this comment.
So, the only reason you added support for both ' and " was to show off you improving RegExp skills 😛
There was a problem hiding this comment.
Improving! I am honoured :-)
Or perhaps I just cut and pasted it from another part of the code :shame:
There was a problem hiding this comment.
Judiciously copy-pasting is a skill to be proud, not ashamed, of 😛
|
Forgot to mention, this will be a-w-e-s-o-m-e 💃 💯 |
This change will enable people to link to the API docs via their selectors or names, as used in a template. Since the selectors can be quite complex we are not able to get 100% accuracy. Closes angular#16787
petebacondarwin
force-pushed
the
aio-autolink-selectors
branch
from
a612155
to
01498d9
Compare
|
@gkalpak PTAL |
|
You can preview 01498d9 at https://pr19317-01498d9.ngbuilds.io/. |
gkalpak
added
action: merge
|
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. |
This change will enable people to link to the API docs via their selectors
or names, as used in a template.
Since the selectors can be quite complex we are not able to get 100%
accuracy.
Closes #16787