-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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): expand support for projectRoot token to include project.json #18302
feat(core): expand support for projectRoot token to include project.json #18302
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/nx/src/config/workspaces.ts
Outdated
const workspaceRootMatch = /^(\{workspaceRoot\}\/?)/.exec(value); | ||
if (workspaceRootMatch?.length) { | ||
value = value.replace(workspaceRootMatch[0], ''); | ||
} else if (value.includes('{workspaceRoot}')) { |
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 handling for this changed a bit as I noticed a potential bug when doing it. If an option represented a path to a directory, {workspaceRoot}
may be a valid value for that, and this would throw as before we required it to have a /
after it.
e4410ca
to
67a5d78
Compare
67a5d78
to
8a9cdcc
Compare
8a9cdcc
to
588a7cc
Compare
588a7cc
to
32937fc
Compare
32937fc
to
f5ffb28
Compare
f5ffb28
to
f035cb2
Compare
f035cb2
to
62407ab
Compare
62407ab
to
ca37e30
Compare
packages/nx/src/config/workspaces.ts
Outdated
const result: T = { ...defaults }; | ||
for (const key in options) { | ||
// Deep merge options objects, but not arrays. | ||
if ( | ||
typeof options[key] === 'object' && | ||
typeof defaults?.[key] === 'object' && | ||
!Array.isArray(options[key]) && | ||
!Array.isArray(defaults?.[key]) | ||
) { | ||
result[key] = mergeOptions(defaults[key], options[key]); | ||
} else { | ||
// The types are either: | ||
// - primitive (string, number, boolean, etc.) | ||
// - arrays | ||
// - different types (e.g. string vs array) | ||
// In any case, we want to override the default with the options. | ||
result[key] = options[key]; | ||
} | ||
} | ||
return result; |
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.
We should not have deep-merging in this PR. It should be separate
packages/nx/src/config/workspaces.ts
Outdated
const workspaceRootMatch = /^(\{workspaceRoot\}\/?)/.exec(value); | ||
if (workspaceRootMatch?.length) { | ||
value = value.replace(workspaceRootMatch[0], ''); | ||
} else if (value.includes('{workspaceRoot}')) { |
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 needs to remain 2 if blocks
ca37e30
to
9feb37b
Compare
9feb37b
to
f5e5b46
Compare
f5e5b46
to
c5bfeaa
Compare
nice! thank you for your work on this. i was running into this exact challenge earlier in the day with |
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. |
Current Behavior
{projectRoot}
,{projectName}
, and{workspaceRoot}
only work inside options or configurations blocks as part oftargetDefaults
innx.json
, but works ininputs
/outputs
inproject.json
already.Expected Behavior
{projectRoot}
,{projectName}
, and{workspaceRoot}
only work insideinputs
,outputs
,options
, andconfigurations
everywhere.Related Issue(s)
Fixes #13272