feat(core): move runtime-lint-utils to eslint plugin #13222
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
meeroslav
force-pushed
the
move-runtime-utils
branch
from
3c96585
to
8856865
Compare
docs/generated/devkit/index.md
Outdated
| @@ -104,6 +104,7 @@ It only uses language primitives and immutable objects | |||
| ### Utils Type aliases | |||
|
|
|||
| - [AdditionalSharedConfig](../../devkit/index#additionalsharedconfig) | |||
| - [MappedProjectGraph](../../devkit/index#mappedprojectgraph) | |||
There was a problem hiding this comment.
Let's not export these publically.
| * @param projectGraph | ||
| * @returns | ||
| */ | ||
| export function mapProjectGraphFiles<T>( |
There was a problem hiding this comment.
We have some really similar methods. Can we use that instead?
It's more widely used and I'm pretty sure its fast.
If you'd really like to keep this version, can you put it next to those and leave a comment to figure out which one is better?
There was a problem hiding this comment.
These two are doing different things.
- The
createProjectRootMappingscreates a map ofprojectRootPath->projectName. This can potentially be replaced bymapProjectGraphFiles. I would do that in a separate PR though. - The
mapProjectGraphFilesadds a fast lookup to graph in the form offilePath->projectName. Keeping this version is vital for the performance of linter rule.
Moving it to target-project-locator makes sense though.
| projectGraph.nodes as Record<string, ProjectGraphProjectNode> | ||
| ).forEach(([name, node]) => { | ||
| node.data.files.forEach(({ file }) => { | ||
| const fileName = removeExt(file); |
There was a problem hiding this comment.
This is a weird way to make a key. Why is it not the dirname?
There was a problem hiding this comment.
Because our imports get resolved to /path/to/folder/file and lookup gets invoked 1000s of times during the lint run.
We need a lookup that's as fast as possible. Any runtime concatenation or extension stripping adds seconds to the total run in the large monorepos.
|
|
||
| return { | ||
| ...projectGraph, | ||
| allFiles, |
There was a problem hiding this comment.
I would just return this separately rather than adding it onto the projectGraph
There was a problem hiding this comment.
That makes sense but would involve changes in linter rule, so I will move that to a followup PR.
meeroslav
changed the title
)" This reverts commit ab6f62a.
)" This reverts commit ab6f62a.
)" This reverts commit ab6f62a.
)" This reverts commit ab6f62a.
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Move runtime-lint-utils from workspace to eslint-plugin-nx and nx
Current Behavior
Expected Behavior
Related Issue(s)
Fixes #11937