-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(core): support targets with colons in the name without quotes #13938
feat(core): support targets with colons in the name without quotes #13938
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
80833cd
to
bde472f
Compare
bde472f
to
9bfcfa8
Compare
const parsedBrowserTarget = parseTargetString(options.browserTarget); | ||
const parsedBrowserTarget = parseTargetString( | ||
options.browserTarget, | ||
readCachedProjectGraph() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readCachedProjectGraph() | |
context.projectGraph |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't exist in this one... Its an angular builder.
...gular/src/migrations/update-12-3-0/convert-webpack-browser-build-target-to-delegate-build.ts
Outdated
Show resolved
Hide resolved
...gular/src/migrations/update-12-3-0/convert-webpack-browser-build-target-to-delegate-build.ts
Outdated
Show resolved
Hide resolved
packages/cypress/src/migrations/update-15-0-0/update-cy-mount-usage.ts
Outdated
Show resolved
Hide resolved
packages/react/src/generators/cypress-component-configuration/lib/add-files.ts
Outdated
Show resolved
Hide resolved
packages/storybook/src/generators/configuration/util-functions.ts
Outdated
Show resolved
Hide resolved
bf81b6a
to
874d214
Compare
874d214
to
475d2ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is good. Two small things:
- We should not change the interface of a core object just for tests.
- Loops in tests is a bad idea.
@@ -28,7 +28,7 @@ export class ProjectGraphBuilder { | |||
/** | |||
* Adds a project node to the project graph | |||
*/ | |||
addNode(node: ProjectGraphProjectNode): void { | |||
addNode<T extends ProjectGraphProjectNode>(node: T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this required ?
@@ -1,14 +1,44 @@ | |||
import { ProjectGraph, ProjectGraphProjectNode } from '../config/project-graph'; | |||
import { ProjectConfiguration } from '../config/workspace-json-project-json'; | |||
import { ProjectGraphBuilder } from '../project-graph/project-graph-builder'; | |||
import { splitTarget } from './split-target'; | |||
|
|||
const cases = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the loop. inline everything.
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. |
See #13914