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.

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: update lab 2 to nx 19.7 #37

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: update lab 2 to nx 19.7 #37

wants to merge 5 commits into from

Conversation

llwt
Copy link
Contributor

@llwt llwt commented Sep 18, 2024

  • bump nx from 19.7.2 to 19.7.4
  • Migration Utility Changes:
    • use new lab-examples generator to copy files into place
  • Lab changes:
    • use nx add to install react plugin
    • use lab-examples generator rather than manually copying files over

Copy link

nx-cloud bot commented Sep 18, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 9ae3089. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added inkscape SVG in case we need to tweak this in the future

Comment on lines +90 to +108
/**
* TODO: figure out what best approach for getProjects(tree) is
*
* Right now, if you call the removeGenerator for a project and then
* immediately call getProjects(tree), getProjects will throw.
*
* This is due to getProjects fetching a list of project.json files
* using globWithWorkspaceContextSync, which globs against the real
* filesystem rather than the in-memory tree.
*
* Once the list of project.json flies is fetched, the project details
* are they attempted to be fetched from the in-memory tree, which
* subsequently fails because we just deleted it.
*
* See: {@link https://github.com/nrwl/nx/blob/master/packages/nx/src/generators/utils/project-configuration.ts#L251}
*
* Re-enabling this test will recreate the issue as long as the current
* workspace has at least one project in it (which the previous test will create)
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reached out to @FrozenPandaz about this and will address it in a future update once we figure out what to do with getProjects(tree).

For now, we just need to run the "reset workspace" migration separately from all the rest.

Comment on lines +53 to 57
{
"input": "./examples",
"glob": "**/*",
"output": "./examples"
}
Copy link
Contributor Author

@llwt llwt Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the example files available for use in generators and migrations allow us to remove the duplicate code currently inlined into the migrations.

Comment on lines +14 to +20
for (const [projectName] of getProjects(tree)) {
await removeGenerator(tree, {
projectName,
skipFormat: true,
forceRemove: true,
});
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we pass forceRemove, we don't need to enumerate the projects and delete them in a specific order.

@@ -33,166 +27,10 @@ export default async function update(tree: Tree) {
style: 'scss',
unitTestRunner: 'jest',
routing: true,
addPlugin: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensures we use crystallized config which is what the user will see when they run the commands directly.

@llwt llwt requested a review from meeroslav September 19, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant