Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(core): Adding react apps/libs to workspaces so they can be referenced. #29202

Merged
merged 3 commits into from
Dec 18, 2024
Merged

fix(core): Adding react apps/libs to workspaces so they can be referenced. #29202

merged 3 commits into from
Dec 18, 2024

Conversation

ndcunningham
Copy link
Contributor

@ndcunningham ndcunningham commented Dec 4, 2024 •
edited
Loading

Current Behavior

This PR address a few things:

  1. When we generate an app or a library using the new TS Solution they are not added to the workspaces/packages option.
  2. Currently, when we use pnpm to generate a workspace the pnpm-workspace.yaml looks like below which is not correct
packages: 
  - packages/**

Expected Behavior

  1. Libraries and apps should be referenced correctly out of the box so that they can be symlinked after installation.
  2. The intended pnpm-workspace.yaml should look similar to:
packages: 
  - 'packages/**'
  - 'demo'

Related Issue(s)

@ndcunningham ndcunningham self-assigned this Dec 4, 2024
@vercel Vercel
Copy link

vercel bot commented Dec 4, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Dec 18, 2024 4:53pm

@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 2 times, most recently from 3b4f6b6 to 47956db Compare December 5, 2024 18:46
@ndcunningham ndcunningham marked this pull request as ready for review December 5, 2024 18:47
@ndcunningham ndcunningham requested a review from jaysoo December 5, 2024 18:47
@ndcunningham ndcunningham changed the title fix(core): React workspaces that use pnpm & the new TS solution should be declared correctly so that it can be referenced fix(core): Adding react apps/libs to workspaces so they can be referenced. Dec 5, 2024
@ndcunningham ndcunningham added the scope: react Issues related to React support for Nx label Dec 5, 2024
@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 4 times, most recently from de51b34 to 52e55e0 Compare December 7, 2024 00:20
leosvelperez
leosvelperez requested changes Dec 9, 2024
View reviewed changes
e2e/utils/create-project-utils.ts Outdated Show resolved Hide resolved
e2e/react/src/react-ts-solution.test.ts Outdated Show resolved Hide resolved
packages/react/src/utils/add-app-to-pnpm-workspace.ts Outdated Show resolved Hide resolved
@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 4 times, most recently from 664eefc to 47cb548 Compare December 9, 2024 17:30
@ndcunningham ndcunningham requested a review from a team as a code owner December 9, 2024 17:30
@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 4 times, most recently from a8a3c6f to 39e31a3 Compare December 9, 2024 21:27
@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch from 39e31a3 to 4c07277 Compare December 10, 2024 07:59
@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 4 times, most recently from 2ed13f1 to b7642d3 Compare December 11, 2024 20:33
@jaysoo jaysoo force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch from b7642d3 to d9242ee Compare December 12, 2024 13:18
@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 4 times, most recently from a807d81 to 1588b1a Compare December 13, 2024 16:39
@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Dec 17, 2024 •
edited
Loading

Your CI Pipeline Execution ↗ for commit 2f3a288 is in progress ⏳

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 43m 49s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 57s View ↗
nx-cloud record -- nx format:check --base=4b586... ✅ Succeeded 21s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded <1s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 18s View ↗
nx documentation --no-dte ✅ Succeeded 37s View ↗
nx affected -t e2e-macos-local --parallel=1 --b... ✅ Succeeded 20m 41s View ↗

☁️ Nx Cloud last updated this comment at 2024-12-18 17:36:27 UTC

@ndcunningham ndcunningham force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 2 times, most recently from 4f52f0b to 0ec0be9 Compare December 18, 2024 08:56
@jaysoo jaysoo force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch from 0ec0be9 to 8fab2e7 Compare December 18, 2024 13:40
@nrwl nrwl deleted a comment from nx-cloud bot Dec 18, 2024
@jaysoo jaysoo force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch 3 times, most recently from 71e320e to 31cf1a6 Compare December 18, 2024 16:12
@jaysoo jaysoo force-pushed the fix/ts-solution-pnpm-workspace-yaml-referrence branch from 31cf1a6 to 2f3a288 Compare December 18, 2024 16:46
@jaysoo jaysoo enabled auto-merge (squash) December 18, 2024 17:31
@jaysoo jaysoo disabled auto-merge December 18, 2024 17:39
@jaysoo jaysoo merged commit d05f30f into master Dec 18, 2024
6 checks passed
@jaysoo jaysoo deleted the fix/ts-solution-pnpm-workspace-yaml-referrence branch December 18, 2024 17:39
ndcunningham added a commit that referenced this pull request Dec 20, 2024
@ndcunningham @jaysoo
fix(core): Adding react apps/libs to workspaces so they can be refere…
ff349c4 
…nced. (#29202)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
This PR address a few things:

1. When we generate an app or a library using the new TS Solution they
are not added to the workspaces/packages option.
2. Currently, when we use `pnpm` to generate a workspace the
`pnpm-workspace.yaml` looks like below which is not correct
```
packages: 
  - packages/**
```

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

1. Libraries and apps should be referenced correctly out of the box so
that they can be symlinked after installation.
2. The intended `pnpm-workspace.yaml` should look similar to:
```
packages: 
  - 'packages/**'
  - 'demo'
```

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

---------

Co-authored-by: Jack Hsu <jack.hsu@gmail.com>
@github-actions GitHub Actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@leosvelperez leosvelperez leosvelperez approved these changes

@jaysoo jaysoo jaysoo approved these changes

@meeroslav meeroslav Awaiting requested review from meeroslav meeroslav is a code owner

@vsavkin vsavkin Awaiting requested review from vsavkin vsavkin is a code owner

@mandarini mandarini Awaiting requested review from mandarini mandarini is a code owner

@Coly010 Coly010 Awaiting requested review from Coly010 Coly010 is a code owner

Assignees

@ndcunningham ndcunningham

Labels
scope: react Issues related to React support for Nx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ndcunningham @jaysoo @leosvelperez