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

NX Packages without default export are excluded from dependencies #29486

Closed
1 of 4 tasks
JacobLey opened this issue Dec 30, 2024 · 4 comments · Fixed by #29577
Closed
1 of 4 tasks

NX Packages without default export are excluded from dependencies #29486

JacobLey opened this issue Dec 30, 2024 · 4 comments · Fixed by #29577
Assignees
Labels
scope: core core nx functionality type: bug

Comments

@JacobLey
Copy link
Contributor

Current Behavior

When a project declares an exports field and does not include a "." export, it will be ignored as a dependency.

e.g.

// foo/project.json
{
    "name": "foo",
    "exports": {
        "./foo": "./foo.js"
    }
}

and

// bar/project.json
{
    "name": "bar",
    "dependencies": {
       "bar": "workspace:^"
    }
}

Nx will not respect the dependency that bar has on foo, despite the explicit declaration in package.json.

This is reflected in both the Web UI and graph output, as well as he dependencies field for project graph from @nx/devkit.

import { createProjectGraphAsync } from '@nx/devkit';

const projectGraph = await createProjectGraphAsync();

projectGraph.dependencies['bar']
// [{ source: 'bar', target: 'npm:foo', type: 'static' }]
// The `npm:` prefix indicates an external dependency, despite it being a local dependency

Expected Behavior

When projects depend on each otheir via package.json (pnpm's "workspace:^" specifier is used as an example, but is hardly the limit), Nx should respect that dependency when building the project graph.

That means that the project graph should explicitly call out the dependency as seen the web UI (or a JSON output), and the dependencies field for project graph from @nx/devkit should also declare the project as a local dependency.

e.g. for example bar's dependency on foo above

import { createProjectGraphAsync } from '@nx/devkit';

const projectGraph = await createProjectGraphAsync();

projectGraph.dependencies['bar']
// [{ source: 'bar', target: 'foo', type: 'static' }]

GitHub Repo

https://github.com/JacobLey/issue-recreator/tree/nx-exports-dependency

Steps to Reproduce

  1. Create a project ("foo") that includes an exports field in the package.json. Export any path other than "." (I suspect this issue would also occur if the exports object is empty)
  2. Create a second project ("bar") that depends on the first. This dependency can be via traditional dependencies field
  3. Run nx graph and inspect the apparent lack of dependency of foo from bar.

Nx Report

Node           : 22.12.0
OS             : linux-arm64
Native Target  : aarch64-linux
pnpm           : 9.15.2

Failure Logs

No explicit failure logs.

Just omission of project as dependency.

Any "failures" would manifest because of tasks being executed out of order and dependencies not being available

Package Manager Version

9.15.2

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

Context is also shared in the issue recreation, which helps demonstrate the fact that this dependency is only omitted when using exports that don't include the "." field.

Repeated here for simplicity:

The logic to declare a dependency as "local" as opposed to "external" is implemented in this line.

const localProject = targetProjectLocator.findDependencyInWorkspaceProjects(d);

The implementation logic for this method is fairly straightforward.
If the name of the project exists in the map, return in it

return this.packageEntryPointsToProjectMap[dep]?.name ?? null;

However it will also calculate this map (memoized), which exposes the bug:

if (!packageExports || typeof packageExports === 'string') {
    // no `exports` or it points to a file, which would be the equivalent of
    // an '.' export, in which case the package name is the entry point
    result[packageName] = project;
} else {
    for (const entryPoint of Object.keys(packageExports)) {
        result[join(packageName, entryPoint)] = project;
    }
}

result['foo'] will get written because join('foo', '.') returns foo.
result['foobar'] would also work because it lacks an exports field.

Howecer result['bar/bar'] is the result of bar because it only has an export for that path.
But the dependency is checked against 'bar', and therefore appears to be an external dependency.

@JacobLey
Copy link
Contributor Author

Image Image

Screenshots of Web UI helping show failure to detect dependency

@JacobLey
Copy link
Contributor Author

Issue originally discovered because a local plugin for my Nx repo suddenly stopped being picked up as a dependency, resulting in failures.

My workaround was to declare a ".": "https://github.com/nrwl/nx/issues/29486" export

@JacobLey
Copy link
Contributor Author

Issue arose between 20.2.2 -> 20.3.0 (can't reproduce on 20.2.2)

Most likely aligning with creation of file

@yosefbs-cy
Copy link

Similar bug when export is define like that:
"exports": {
"import": "./dist/index.js",
"require": "./dist-cjs/index.js",
"default": "./dist/index.js"
},

then in:
https://github.com/nrwl/nx/blob/master/packages/nx/src/plugins/js/utils/packages.ts#L23

the packages that import it doesn't recognize as depend on that package.

@leosvelperez

@leosvelperez leosvelperez added the scope: core core nx functionality label Jan 9, 2025
@leosvelperez leosvelperez self-assigned this Jan 9, 2025
@jaysoo jaysoo closed this as completed in d08ad75 Jan 10, 2025
jaysoo pushed a commit that referenced this issue Jan 10, 2025
#29577)

Sibling dependencies that rely exclusively on subpath exports are
excluded from the dependency graph because there is no exact match.

This adds a fallback to look for subpath exports if the exact match is
not found.

This also adds logic to respect conditional exports independent from
subpath exports.

<!-- 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

Importing a workspace dependency via subpath export fails to match the
package name, and so is not included in the dependency graph.

### Example

```apps/api/package.json```
```json
  "name": "@my-org/api",
  "dependencies": {
    "@my-org/services": "workspace:*"
  }
```

```libs/services/package.json```
```json
  "name": "@my-org/services",
  "exports": {
    "./email": "./dist/email.js"
  }
```

The `@my-org/api` app should be able to import the email service with
`import { EmailService } from "@my-org/services/email"`.

However, the `getPackageEntryPointsToProjectMap` implementation results
in an object with a key of `@my-org/services/email`, but not
`@my-org/services`. This is not specifically a problem, except that
`findDependencyInWorkspaceProjects` only considers exact matches within
those object keys.

## Expected Behavior

Importing a workspace dependency via subpath export should be included
in the dependency graph.

I also addressed a related issue where the following resulted in keys of
`@my-org/services/default` and `@my-org/services/types`, which is
incorrect according to the subpath/conditional export rules.
```json
  "exports": {
    "default": "./dist/index.js",
    "types": "./dist/index.d.ts"
  }
```

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

Fixes #29486

---------

Co-authored-by: Leosvel Pérez Espinosa <[email protected]>
FrozenPandaz pushed a commit that referenced this issue Jan 15, 2025
#29577)

Sibling dependencies that rely exclusively on subpath exports are
excluded from the dependency graph because there is no exact match.

This adds a fallback to look for subpath exports if the exact match is
not found.

This also adds logic to respect conditional exports independent from
subpath exports.

<!-- 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

Importing a workspace dependency via subpath export fails to match the
package name, and so is not included in the dependency graph.

### Example

```apps/api/package.json```
```json
  "name": "@my-org/api",
  "dependencies": {
    "@my-org/services": "workspace:*"
  }
```

```libs/services/package.json```
```json
  "name": "@my-org/services",
  "exports": {
    "./email": "./dist/email.js"
  }
```

The `@my-org/api` app should be able to import the email service with
`import { EmailService } from "@my-org/services/email"`.

However, the `getPackageEntryPointsToProjectMap` implementation results
in an object with a key of `@my-org/services/email`, but not
`@my-org/services`. This is not specifically a problem, except that
`findDependencyInWorkspaceProjects` only considers exact matches within
those object keys.

## Expected Behavior

Importing a workspace dependency via subpath export should be included
in the dependency graph.

I also addressed a related issue where the following resulted in keys of
`@my-org/services/default` and `@my-org/services/types`, which is
incorrect according to the subpath/conditional export rules.
```json
  "exports": {
    "default": "./dist/index.js",
    "types": "./dist/index.d.ts"
  }
```

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

Fixes #29486

---------

Co-authored-by: Leosvel Pérez Espinosa <[email protected]>
(cherry picked from commit d08ad75)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: core core nx functionality type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants