sverweij / dependency-cruiser

Validate and visualize dependencies. Your rules. JavaScript, TypeScript, CoffeeScript. ES6, CommonJS, AMD.
https://npmjs.com/dependency-cruiser
MIT License
5.15k stars 249 forks source link

Issue: cruise API - breaking changes #932

Closed AdrienLeblanc closed 4 months ago

AdrienLeblanc commented 4 months ago

Hello ! Hope you're doing well.

Context

I had the chance to implement a little Vite Plugin running a circular deps check on fileq that are hot reloaded in my project. I decided to use your tool and it works as a charm in command-line. However, I encountered some problems with the API, since my goal is to use it programmatically.

I believe/suppose they are due to previous breaking changes of your project.

Current Behavior

  1. The utilitary function extractDepcruiseConfig that is supposed to extract .dependency-cruiser.js config file works, but the resulting object does not match the ICruiseOptions interface. For example, the ruleSet property isn't set, and rules (forbidden, allowed, etc) are extracted to { forbidden: [...] } instead of { ruleSet: { forbidden: [...] } }. The same goes for doNotFollow, probably other options.
  2. I had to set cruiseOptions.validate = true in order to have a dependency check performed. Once done, it couldn't work (undefined error) unless I set cruiseOptions.ruleSet.allowed = []. This was not required using the command-line interface.
  3. I couldn't have my tsconfig.json be taken into account while performing the dependency checks, I figured out it was due to an interface problem with the cruise() function. See below
    const cruiseResult = await cruise([path], cruiseOptions, {
    tsConfig: './tsconfig.json'
    });

    This code sample works, but does not match the interface nor your documentation found at https://github.com/sverweij/dependency-cruiser/blob/main/doc/api.md

Expected Behavior

The extractDepcruiseConfig util should structure the resulting object accordingly to the ICruiseOptions interface. I believe the option validate = true shouldn't be required to perform the cruise. The cruise entrypoint should recognize the tsconfig.json file if specified in cruiseOptions or as an object provided as 4th parameter, as intended in the documentation.

Code sample

My project is work related, but if necessary I guess I could provide more but for now here's how I use your tool :)

const path = relative(__dirname, file);
const depcruiseConfig: ICruiseOptions = await extractDepcruiseConfig('./.dependency-cruiser.js');
const cruiseOptions: ICruiseOptions = {
        ruleSet: {
            forbidden: depcruiseConfig['forbidden'],
            allowed: []
        },
        doNotFollow: {
            path: ['node_modules']
        },
        enhancedResolveOptions: {
            exportsFields: ['exports'],
            conditionNames: ['import', 'require', 'node', 'default', 'types'],
            mainFields: ['main', 'types', 'typings']
        },
        tsPreCompilationDeps: true,
        combinedDependencies: true,
        validate: true
};
try {
        const cruiseResult = await cruise([path], cruiseOptions, {
            tsConfig: './tsconfig.json'
        });
        const output = cruiseResult.output as ICruiseResult;
        if (output.summary.violations.length > 0) {
            printViolations(output.summary.violations);
        }
} catch (error) {
        console.error(error);
}

Your Environment

sverweij commented 4 months ago

Hi @AdrienLeblanc thanks for raising this bug.

Indeed currently extractDepcruiseConfig returns an IConfiguration object while the cruise function needs an ICruiseOptions .

validate=true isn't needed to run a cruise, but it is necessary if dependency-cruiser is to validate the cruised modules against rules. This is indeed different from the cli which assumes defaults to validating (since version 13.0.0).

sverweij commented 4 months ago

Hi @AdrienLeblanc - thanks for your patience; today I finally found some space to fix this - see that linked pull request above (which also contains an example).

It's currently releasing to npmjs as part of dependency-cruiser@16.3.2

Let me know if there's still issues

AdrienLeblanc commented 4 months ago

Hello @sverweij

Thanks for the update. The depcruise options extraction works like a charm now. However, I still encounter some issue with the intended way of passing the tsconfig.json as argument to the cruise function.

Here's my implementation when I try to use the intended way (I do not use Webpack)

const depcruiseOptions: ICruiseOptions = await extractDepcruiseOptions('./.dependency-cruiser.js');
const tsConfig = extractTSConfig("./tsconfig.json");
const cruiseResult = await cruise(['src/test.vue'], depcruiseOptions, null, { tsConfig });

I suspect that the tsconfig.json isn't properly used/read with this sample, since it doesn't spot an obvious circular dependency I placed on purpose in the file src/test.vue (which is spotted using the CLI)

The only solution that I found for the moment is to implement it as below :

const depcruiseOptions: ICruiseOptions = await extractDepcruiseOptions('./.dependency-cruiser.js');
const cruiseResult = await cruise(jsFiles, depcruiseOptions, { tsConfig: './tsconfig.json' });

I omit the third argument and indicate the path to my tsconfig.json as a string.

Am I missing something maybe ? Thanks in advance for your time

sverweij commented 4 months ago

Hi @AdrienLeblanc I've created a minimal reproduction repo for this new issue (see dependency-cruiser-repro-repo/932 ~, but failed to reproduce it.~ and found the likely cause; see annotated source below.

import { cruise } from 'dependency-cruiser';
import extractDepcruiseOptions from "dependency-cruiser/config-utl/extract-depcruise-options";
import extractTSConfig from "dependency-cruiser/config-utl/extract-ts-config";

/** @type {ICruiseOptions} */
const depcruiseOptions = await extractDepcruiseOptions('./.dependency-cruiser.js');
const tsConfig = extractTSConfig("./tsconfig.json");

const cruiseResult = await cruise(
    // files/ folders to cruise
    ['src/'], 

    // cruise options
    depcruiseOptions, 

    // resolve options
    // we strive to have everything in memory (except for the files be cruised).
    // however it's  not possible pass a (parsed) tsconfig to tsconfig-paths;
    // it needs the file name again. We could've solved  this behind the API,
    // but ended up doing that in the CLI, so here it needs to be passed twice
    { tsConfig: "./tsconfig.json" },

    // transpiler options
    // passing the parsed tsConfig in transpiler options is still recommended, 
    // though as it will influence how typescript interprets/ transpiles your
    // code.
    { tsConfig }
);

console.dir(cruiseResult.output.summary, { depth: null});