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

fix(node): Enable e2e test #23508

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(node): Enable e2e test #23508

wants to merge 1 commit into from

Conversation

ndcunningham
Copy link
Contributor

@ndcunningham ndcunningham commented May 17, 2024

Re-enables e2e tests for node

Copy link

vercel bot commented May 17, 2024

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

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview May 27, 2024 9:59pm

@ndcunningham ndcunningham force-pushed the fix/node-e2e-test branch 2 times, most recently from be7fcdc to 419e05c Compare May 18, 2024 02:38
@ndcunningham ndcunningham self-assigned this May 18, 2024
@ndcunningham ndcunningham added the scope: node Issues related to Node, Express, NestJS support for Nx label May 18, 2024
@ndcunningham ndcunningham marked this pull request as ready for review May 18, 2024 02:50
@ndcunningham ndcunningham requested a review from a team as a code owner May 18, 2024 02:50
@ndcunningham ndcunningham requested a review from jaysoo May 18, 2024 02:50
@@ -30,15 +28,16 @@ function normalizeOptions(
}

function addDocker(tree: Tree, options: SetUpDockerOptions) {
// Inferred targets are only available in the project graph
const projectConfig = readCachedProjectConfiguration(options.project);
const projectConfig = readProjectConfiguration(tree, options.project);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't include inferred targets...

Shouldn't we still be able to setup docker for projects with inferred build targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not?

  1. If the project has not been created yet, reading from the graph would be invalid right?
  2. The tree is passed in as an argument, which is probably the source of truth for generators.

The current setup should work for both crystal and non-inferred projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: node Issues related to Node, Express, NestJS support for Nx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants