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

Question: Rule for forbidding imports from above an entrypoint file location? #960

Closed
cdanielsen opened this issue Oct 23, 2024 · 8 comments
Labels

Comments

@cdanielsen
Copy link

Summary

I have a need for a rule I might call something like no-root-folder-ancestor (see context below), that I sense is possible with the forbidden.from/to apis, but my weakness with regex is preventing me from achieving it. I've created a repo below with a minimal reproduction case - would really appreciate any thoughts or suggestions that might get at what I'm after. Thank you, this project is really cool!

Context

I'd like this rule to help me with a project involving migrating files from a large frontend repository with a monolith architecture (everything in one src folder) to a monorepo style (many individual packages). In order to move a chunk of code and the tree of files underneath it from the src folder into a package, I need to understand/mitigate any imports that are reaching out beyond the boundaries of that chunk of code , because packages should be self contained / cannot import things from the old monolith (only other packages).

Environment

@cdanielsen
Copy link
Author

Somewhat similar to #330

@sverweij
Copy link
Owner

sverweij commented Oct 27, 2024

Hi @cdanielsen given your use case I assume you don't only want to check/ forbid code reaching out to parent folders, but also to siblings of the root folder (like src/otherRootFolder/thing.js)?

I've made a PR to your minimal reproduction example (thanks for that!) that provides a way to do that, larded with comments that could be helpful for tweaking it to your context: https://github.com/cdanielsen/dep-cruiser-no-ancestor/pull/1/files

Let me know if you have additional questions (or if I misunderstood your question)

@cdanielsen
Copy link
Author

cdanielsen commented Oct 28, 2024

Hi @sverweij, thanks for the PR. That worked great in the reproduction repo, but in the repo I'd actually like to apply that rule to, I seem to have some difference in configuration that's causing dep-cruiser not to resolve relative paths? e.g. when I run it on the little reproduction repo I get:

error no-root-folder-ancestor: src/rootFolder/subFolder/sub-root.js → src/outside-root.js

Where the violation import path is fully resolved (src/outside-root.js) even though in the file the statement is

import { exportFromOutsideRoot } from '../../outside-root.js';

But when I run it in the other repo I get stuff like this

  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ./visitor-link-expiration
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ./default-allow-anonymous-editors
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ./default-allow-anonymous-alias
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ./allow-anonymous-editors
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ./allow-anonymous-alias
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ../../common/description
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ../../common/control-with-nested-content
  error no-outside-root: src/company/settings/general/allow-visitor-invitations/index.tsx → ../../../../views/team/settings/visitors-settings/link-permissions/limited-member-upgrade

where the import paths are NOT fully resolved / I'm guessing why they all being flagged (none of them start with src/).

This project is much different than the little reproduction repo in that it uses tools like typescript and babel, but I did generate a config using the --init option to detect those things? Is there some config that might be causing it not to resolve those import statements? Thanks again!

@cdanielsen
Copy link
Author

Hi @cdanielsen given your use case I assume you don't only want to check/ forbid code reaching out to parent folders, but also to siblings of the root folder (like src/otherRootFolder/thing.js)?

Just to clarify too, yes, this is correct (no siblings as well as ancestors, so your no-outside-root naming was more accurate for my use case)

@sverweij
Copy link
Owner

sverweij commented Oct 28, 2024

Hi @cdanielsen it indeed seems like it's resolving less than it should.

One hypothesis is the options.enhancedResolveOptions does currently contain all values needed for your context.

--init uses some heuristics to e.g. guess/ find the extensions used in your project and writes the minimal set it thinks it can get away with to the .dependency-cruiser.js. It does so because trying to resolve just 3 extensions vs all potentially 18 of them is much faster. Drawback is that when the heuristic isn't spot on it'll miss extensions and files won't get resolved. Commenting out the line with extensions will ensure that dependency-cruiser (/ enhanced-resolve - the resolver it uses) takes into account all extensions associated to compilers it has available.

Another (less likely) hypothesis is that the "mainFields" needs to be adjusted as per the generated comment

      // if you migrate to ESM (or are in an ESM environment already) you will want to
      // have "module" in the list of mainFields, like so:
      // mainFields: ["module", "main", "types", "typings"],
      mainFields: ["main", "types", "typings"],

If these suggestions don't work, can you share the options section of the .dependency-cruiser.js in use on the other repo?

@cdanielsen
Copy link
Author

It looks like enhancedResolveOptions.extensions was already commented out from --init, and I got the same output when adding "module" to the mainFields array =/ Here's the full config if anything else jumps out/appreciate you taking a look:

/** @type {import('dependency-cruiser').IConfiguration} */
module.exports = {
  forbidden: [
    {
      name: 'no-outside-root',
      comment:
        'Files cannot import files above the entrypoint containing folder',
      severity: 'error',
      from: {
        // Ideally this would capture any files below the entrypoint containing folder

        /* For this _specific_ scenario, only mentioning the folder name directly
         * would also work, but I guess the more generic solution is what you're
         * after
         */
        // path: "^src/(rootFolder/).+"

        /* The more  generic solution is to use `[^/]+/` to match a folder (1 or
         * more characters that are not a /, followed by a /) and then any file
         * in that folder
         */
        path: '^src/([^/]+/).+',
      },
      to: {
        /* If the path in the to object is src/rootFolder, then this would match
         * any file that isn't in src/rootFolder (or a subfolder of it), so
         * both the root (e.g. src/outside-root.js) but also sibling folders
         * (e.g. src/otherRootFolder/sub/anyFile.js) which is probably what you need
         * for your use case
         * The $1 in the pathNot references  the capturing group in the
         * path property (the ([^/]+/) part)
         */
        pathNot: '^src/$1',

        /* Capturing only files in folder that are that are parents of src/rootFolder,
         * but not capturing any files in sibling folders _is_ possible, in that case
         * you'd use:
         */
        // pathNot: "^src/([^/]+/).+"

        /* If it's about moving things around, it could be you're OK to keep 3rd
         * party dependencies ('node_modules') as well as node built-ins around.
         * To allow for that you'd want to add:
         */
        // dependencyTypesNot: ["npm", "core"]
      },
    },
  ],
  options: {
    /* Which modules not to follow further when encountered */
    doNotFollow: {
      /* path: an array of regular expressions in strings to match against */
      path: ['node_modules'],
    },

    /* Which modules to exclude */
    // exclude : {
    //   /* path: an array of regular expressions in strings to match against */
    //   path: '',
    // },

    /* Which modules to exclusively include (array of regular expressions in strings)
       dependency-cruiser will skip everything not matching this pattern
    */
    // includeOnly : [''],

    /* List of module systems to cruise.
       When left out dependency-cruiser will fall back to the list of _all_
       module systems it knows of. It's the default because it's the safe option
       It might come at a performance penalty, though.
       moduleSystems: ['amd', 'cjs', 'es6', 'tsd']

       As in practice only commonjs ('cjs') and ecmascript modules ('es6')
       are widely used, you can limit the moduleSystems to those.
     */

    // moduleSystems: ['cjs', 'es6'],

    /* prefix for links in html and svg output (e.g. 'https://github.com/you/yourrepo/blob/main/'
       to open it on your online repo or `vscode://file/${process.cwd()}/` to
       open it in visual studio code),
     */
    // prefix: `vscode://file/${process.cwd()}/`,

    /* false (the default): ignore dependencies that only exist before typescript-to-javascript compilation
       true: also detect dependencies that only exist before typescript-to-javascript compilation
       "specify": for each dependency identify whether it only exists before compilation or also after
     */
    tsPreCompilationDeps: true,

    /* list of extensions to scan that aren't javascript or compile-to-javascript.
       Empty by default. Only put extensions in here that you want to take into
       account that are _not_ parsable.
    */
    // extraExtensionsToScan: [".json", ".jpg", ".png", ".svg", ".webp"],

    /* if true combines the package.jsons found from the module up to the base
       folder the cruise is initiated from. Useful for how (some) mono-repos
       manage dependencies & dependency definitions.
     */
    // combinedDependencies: false,

    /* if true leave symlinks untouched, otherwise use the realpath */
    // preserveSymlinks: false,

    /* TypeScript project file ('tsconfig.json') to use for
       (1) compilation and
       (2) resolution (e.g. with the paths property)

       The (optional) fileName attribute specifies which file to take (relative to
       dependency-cruiser's current working directory). When not provided
       defaults to './tsconfig.json'.
     */
    tsConfig: {
      fileName: 'tsconfig.json',
    },

    /* Webpack configuration to use to get resolve options from.

       The (optional) fileName attribute specifies which file to take (relative
       to dependency-cruiser's current working directory. When not provided defaults
       to './webpack.conf.js'.

       The (optional) `env` and `arguments` attributes contain the parameters
       to be passed if your webpack config is a function and takes them (see
        webpack documentation for details)
     */
    // webpackConfig: {
    //  fileName: 'webpack.config.js',
    //  env: {},
    //  arguments: {}
    // },

    /* Babel config ('.babelrc', '.babelrc.json', '.babelrc.json5', ...) to use
      for compilation
     */
    babelConfig: {
      fileName: '.babelrc',
    },

    /* List of strings you have in use in addition to cjs/ es6 requires
       & imports to declare module dependencies. Use this e.g. if you've
       re-declared require, use a require-wrapper or use window.require as
       a hack.
    */
    // exoticRequireStrings: [],

    /* options to pass on to enhanced-resolve, the package dependency-cruiser
       uses to resolve module references to disk. The values below should be
       suitable for most situations

       If you use webpack: you can also set these in webpack.conf.js. The set
       there will override the ones specified here.
     */
    enhancedResolveOptions: {
      /* What to consider as an 'exports' field in package.jsons */
      exportsFields: ['exports'],
      /* List of conditions to check for in the exports field.
         Only works when the 'exportsFields' array is non-empty.
      */
      conditionNames: ['import', 'require', 'node', 'default', 'types'],
      /*
         The extensions, by default are the same as the ones dependency-cruiser
         can access (run `npx depcruise --info` to see which ones that are in
         _your_ environment). If that list is larger than you need you can pass
         the extensions you actually use (e.g. [".js", ".jsx"]). This can speed
         up module resolution, which is the most expensive step.
       */
      // extensions: [".js", ".jsx", ".ts", ".tsx", ".d.ts"],
      /* What to consider a 'main' field in package.json */

      // if you migrate to ESM (or are in an ESM environment already) you will want to
      // have "module" in the list of mainFields, like so:
      // mainFields: ["module", "main", "types", "typings"],
      mainFields: ['main', 'types', 'typings', 'module'],
      /*
         A list of alias fields in package.jsons
         See [this specification](https://github.com/defunctzombie/package-browser-field-spec) and
         the webpack [resolve.alias](https://webpack.js.org/configuration/resolve/#resolvealiasfields)
         documentation

         Defaults to an empty array (= don't use alias fields).
       */
      // aliasFields: ["browser"],
    },
    reporterOptions: {
      dot: {
        /* pattern of modules that can be consolidated in the detailed
           graphical dependency graph. The default pattern in this configuration
           collapses everything in node_modules to one folder deep so you see
           the external modules, but their innards.
         */
        collapsePattern: 'node_modules/(?:@[^/]+/[^/]+|[^/]+)',

        /* Options to tweak the appearance of your graph.See
           https://github.com/sverweij/dependency-cruiser/blob/main/doc/options-reference.md#reporteroptions
           for details and some examples. If you don't specify a theme
           dependency-cruiser falls back to a built-in one.
        */
        // theme: {
        //   graph: {
        //     /* splines: "ortho" gives straight lines, but is slow on big graphs
        //        splines: "true" gives bezier curves (fast, not as nice as ortho)
        //    */
        //     splines: "true"
        //   },
        // }
      },
      archi: {
        /* pattern of modules that can be consolidated in the high level
          graphical dependency graph. If you use the high level graphical
          dependency graph reporter (`archi`) you probably want to tweak
          this collapsePattern to your situation.
        */
        collapsePattern:
          '^(?:packages|src|lib(s?)|app(s?)|bin|test(s?)|spec(s?))/[^/]+|node_modules/(?:@[^/]+/[^/]+|[^/]+)',

        /* Options to tweak the appearance of your graph. If you don't specify a
           theme for 'archi' dependency-cruiser will use the one specified in the
           dot section above and otherwise use the default one.
         */
        // theme: { },
      },
      text: {
        highlightFocused: true,
      },
    },
  },
};
// generated: dependency-cruiser@16.5.0 on 2024-10-28T23:21:00.710Z

@sverweij
Copy link
Owner

sverweij commented Nov 3, 2024

Looks like the options are set correctly.

  • What does npx depcruise --info (run from the same location as you'd run dependency-cruiser from) show?
  • src/company/settings/general/allow-visitor-invitations/index.tsx imports ./visitor-link-expiration. What the resolver will do is try a few options:
    • look for a file called visitor-link-expiration in the same folder.
    • Failing that it will run through the extensions it knows (npx depcruise --info will tell which those are), paste them behind the file name and look for that, in the same folder (visitor-link-expiration.js, visitor-link-expiration.cjs, ..., visitor-link-expiration.ts, visitor-link-expiration.tsx, ...).
    • The extensions it uses by default are informed by the compilers it has at its disposal. If it e.g. can't find a typescript compiler it won't paste extensions associated with typescript behind them and skip to the next).
Example: express

E.g. when running depcruise --info in the root of the express repo it'll return the response below.

dependency-cruiser@16.5.0

    node version supported : ^18.17||>=20
    node version found     : v22.10.0
    os version found       : x64 darwin@21.6.0

    If you need a supported, but not enabled transpiler ('x' below), just install
    it in the same folder dependency-cruiser is installed. E.g. 'npm i livescript'
    will enable livescript support if it's installed in your project folder.

    ✔ transpiler             versions supported  version found
    - ---------------------- ------------------- ------------------------
    ✔ javascript             *                   acorn@8.14.0
    ✔ babel                  >=7.0.0 <8.0.0      @babel/core@7.26.0
    x coffee-script          >=1.0.0 <2.0.0      -
    x coffeescript           >=1.0.0 <3.0.0      -
    x livescript             >=1.0.0 <2.0.0      -
    x svelte                 >=3.0.0 <6.0.0      -
    x swc                    >=1.0.0 <2.0.0      -
    x typescript             >=2.0.0 <6.0.0      -
    x vue-template-compiler  >=2.0.0 <3.0.0      -
    x @vue/compiler-sfc      >=3.0.0 <4.0.0      -

    ✔ extension
    - ---------
    ✔ .js
    ✔ .cjs
    ✔ .mjs
    ✔ .jsx
    x .ts
    x .tsx
    x .d.ts
    x .cts
    x .d.cts
    x .mts
    x .d.mts
    x .vue
    x .svelte
    x .ls
    x .coffee
    x .litcoffee
    x .coffee.md
    x .csx
    x .cjsx

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants