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(vite): add angular option to vitest generator #29055

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

yjaaidi
Copy link
Contributor

@yjaaidi yjaaidi commented Nov 25, 2024

Current Behavior

@nx/vite:vitest generator does not provide Angular support in the uiFramework options.

Expected Behavior

angular option should generate the vitest configuration just like @nx/angular:application and @nx/angular/library do.

Copy link

vercel bot commented Nov 25, 2024

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 Nov 26, 2024 6:14pm

@yjaaidi
Copy link
Contributor Author

yjaaidi commented Nov 25, 2024

@nx/vite:vitest's uiFramework should be required but that would be a breaking change. Should I add it here or should we keep this for the next major?

@yjaaidi
Copy link
Contributor Author

yjaaidi commented Nov 25, 2024

In order to improve tests maintainability and symmetry to production, I would love to replace fixtures like these: https://github.com/nrwl/nx/blob/8738d10446f17df7f9f30e3c94e509062feb0df2/packages/vite/src/utils/test-utils.ts

with reusable "object mothers" in testing utils:

const workspace = workspaceMother.withInferenceDisabled().build();
const app = angularAppMother.withName('my-app').withJestSetup().withSomeTests().build();
const lib = angularLibMother.withName('my-lib').build(); // no test config & no tests

createWorkspace(workspace);
createProjects([app, lib])

What do you think?

Copy link
Collaborator

@isaacplmann isaacplmann left a comment

Choose a reason for hiding this comment

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

Docs changes approved

Copy link
Contributor

@Coly010 Coly010 left a comment

Choose a reason for hiding this comment

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

Looks good, and testing it out locally worked fine.

One addressable comment, another is more thought-provoking but doesn't need to be solved.

packages/vite/src/generators/vitest/vitest.spec.ts Outdated Show resolved Hide resolved

if (!schema.skipViteConfig) {
if (schema.uiFramework === 'react') {
if (schema.uiFramework === 'angular') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there is any way we can detect this if the user does not pass it in 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is conceptually challenging.

We could implement heuristics that analyze the imports. The problem is that a project can be defined with a "targeted" type before it has anything in it that is specific to that framework.

Let's say I have an Angular library with nothing Angular-specific yet, it would assume uiFramework: none, and then simply not work once I decide to add something Angular-specific.
Then we would need a generator that updates the configuration according to the detected framework so things can get a bit complicated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.

With angular applications and buildable libraries, we can check the project graph to see if the project has a dependency on any @angular or @nx/angular package.

If they generated their library via @nx/angular:library generator, one of those dependencies should still exist.

But if they generated their library via @nx/js:library then it wouldn’t.

That said, I think that is good enough.

We can try to infer it if someone has not passed in --uiFramework.

Worse case they end up with “none” which is the same outcome as they didn’t pass in --uiFramework.

At the start of the generator we can add something like

schema.uiFramework ??= tryDetermineFramework()

so that it only attempts it if the user has not provided it.

the tryDetermineFramework can use the project graph to look for dependencies.

const REACT_DEPS = [“react”, “react-dom”, ]
const ANGULAR_DEPS = [@angular/core”, @nx/angular”, ]
const projectGraph = await createProjectGraphAsync();

const projectDeps = projectGraph.deps.filter(dep => dep.source === schema.project).reduce((collectedDeps, dep) => collectedDeps.add(dep), new Set())

const isReact = REACT_DEPS.some(dep => projectDeps.has(dep));
const isAngular = ANGULAR_DEPS.some(dep => projectDeps.has(dep))

schema.uiFramework = isReact 
  ? “react” 
  : isAngular
    ? “angular”
     : “none”

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.

3 participants