dependency-checks lint rule does not take "external" setting into account

[ajwootto]

Current Behavior

When using the dependency-checks lint rule to verify source package.json dependencies are listed correctly, the linter incorrectly reports that a package is missing an entry for a workspace package it depends on, if that package is meant to be bundled into the output.

eg. when setting the external setting to a list of packages, any package not listed there will be bundled into the final output, but the linter incorrectly lists it as a missing dependency in the package.json

The rule also does not enforce that the dependencies of that inlined dependency need to be listed in the outer package's package.json. In fact, if you do list them yourself then the linter throws an error that you're listing a dependency that "isn't used" (which is wrong).

Expected Behavior

The linter should understand the "external" setting and not report these as missing dependencies.

Any package that is being inlined should add its set of required dependencies to the list that the lint rule requires for the outer package.

I know you can manually specify overrides, but it feels cumbersome to list out every package dependency that is not listed in the external array, in order for the linter to be happy. It also fails to protect you from not listing required dependencies that are a result of inlining packages.

I think these two parts of Nx should work together correctly.

GitHub Repo

https://github.com/DevCycleHQ/js-sdks/pull/593/commits/f31a476f258311335448401658fb799e8f568e73

Steps to Reproduce

1. Clone repo at linked commit
2. yarn
3. yarn lint nodejs

See the linting error about packages which are bundled into the final output due to external setting

Nx Report

>  NX  Falling back to ts-node for local typescript execution. This may be a little slower.
  - To fix this, ensure @swc-node/register and @swc/core have been installed

 >  NX   Report complete - copy this into the issue template

   Node   : 18.16.0
   OS     : darwin-arm64
   yarn   : 3.6.0

   nx                 : 16.10.0
   lerna              : 5.6.2
   @nx/js             : 16.10.0
   @nx/jest           : 16.10.0
   @nx/linter         : 16.10.0
   @nx/workspace      : 16.10.0
   @nx/cypress        : 16.10.0
   @nx/detox          : 16.10.0
   @nx/devkit         : 16.10.0
   @nx/esbuild        : 16.10.0
   @nx/eslint-plugin  : 16.10.0
   @nx/next           : 16.10.0
   @nx/node           : 16.10.0
   @nx/react          : 16.10.0
   @nx/react-native   : 16.10.0
   @nrwl/rollup       : 16.10.0
   @nx/web            : 16.10.0
   @nx/webpack        : 16.10.0
   typescript         : 5.1.6
   ---------------------------------------
   Community plugins:
   @altack/nx-bundlefy : 0.16.0

Failure Logs

No response

Package Manager Version

No response

Operating System

• [X] macOS
• [ ] Linux
• [ ] Windows
• [ ] Other (Please specify)

Additional Information

No response

[meeroslav]

Hi,

Not sure what you mean by "external" setting, but running the lint on you main branch I can see that packages are detected based on your usage in the graph.

What you are missing is "!{projectRoot}/__tests__/**" in your production named input in nx.json that would filter out dependencies only used for testing and "!{projectRoot}/node_modules/**" that would filter the nested node modules.

Then your package.json dependencies can be shortened to:

{
    ...
    "dependencies": {
        "@devcycle/js-cloud-server-sdk": "^1.3.0",
        "@devcycle/types": "^1.4.0"
    },
    ...
}

and you can also remove all ignores from the .eslintrc's rule

[ajwootto]

Hi @meeroslav I'm talking about the "external" setting available in many of the nx build executors which controls whether to inline the build output of a dependant library inside the parent rather than treat it is as a separate built and published package.

Since the lint rule doesn't understand the concept of "external", it isn't really possible to use it in a logical way when trying to inline certain packages in the workspace. Your suggestion about ignoring test file dependencies doesn't solve this case because I'm talking about packages that are required in actual source files, but which are bundled using the "external" setting.

[ajwootto]

is there any way to at least allow the lint rule to contain a separate list of "internalized" packages so that they can be treated properly without having to fully "ignore" them? Or even better, treat the "external" setting as a first-class Nx concept and update the linter to recognize it automatically?

The same problem exists in the enforce-module-boundaries lint rule: https://github.com/nrwl/nx/issues/20033

[meeroslav]

In short, no. This is because the linter rule is agnostic of all the other targets and their executors.

You could have two build targets defined:

• inline-build which inlines everything
• external-build, which keeps everything external

Both could be used for different purposes. How should the linter rule react in this situation? Should it require linked packages to be added to package.json because of the external build or ignore them because of the inline build?

Moreover, users might be using plain command or third-party executor to run the build in which case we have no way or knowing how the dependencies will be treated.

We strongly believe that "forcing" user to list all their inlined packages in the ignore list is a better solution long term than introducing a potential foot gun by parsing target flags and making assumptions about the build.

[ajwootto]

@meeroslav thanks for the reply

I said this in the other issue but I wonder if the concept of an "external" vs. "internal" package needs to be represented somehow in the project graph rather than making it a build-only concern? It feels like there's a bit of overlap of what information the lint rules need to make correct determinations about dependencies and what the build stage is actually going to do. Basically making the concept of an "internal" package exist at the project.json root level rather than the build executor level, making the scenario you describe about having different build targets with different external settings "impossible"

Aside from that, about the ignore list in the lint rule: I made a related argument in this issue https://github.com/nrwl/nx/issues/20070

but basically I think it's potentially a problem to only have the concept of a "fully ignored" list, since that turns off all checks about that dependency. In this case it would be more powerful to have an "inlined" list option, that basically tells the linter "don't expect this package to be listed in the package.json, but do expect all of its dependencies to be listed". Otherwise there's nothing protecting you from failing to list the dependencies that the inlined dependencies use, which feels like a big pitfall in a complex repo.

[github-actions[bot]]

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.