Question: can I force dep-cruiser to resolve `.browser.js` files over regular `.js` files? about dependency-cruiser HOT 12 CLOSED

Hi @JakeSidSmith thanks again for your help - the addition is part of the freshly released [email protected]

Regarding the webpackConfig/ ResolveOptions - the options specified in .dependency-cruiser.js config files indeed only give the option to specify a webpack configuration file and nothing else. The main function, however takes an (expanded) webpack configuration object with all bells and whistles. It looks like this is not properly reflected in the typings of the main function - something that needs to be fixed as well. I have other reasons to take a closer look at the typings soon, and I'll try to rectify that at that time as well.

from dependency-cruiser.

Well, I think I've figured it out. After some hunting I discovered that engine.io provides a browser field in the package.json (https://github.com/search?q=repo%3Asocketio%2Fengine.io-client%20xmlhttprequest.browser.js&type=code).

Which works in the same way (as far as I can tell) as an exports field, so simply updating my config to the following has resolved my issues:

exportsFields: ['browser', 'exports'],

from dependency-cruiser.

Nope, now I have additional issues. 😂

Fails to resolve @emotion/is-prop-valid from react-jss.

@emotion/is-prop-valid also defines a browser field, and my new config seems to prefer resolving the browser field over the module field, even though the import is of @emotion/is-prop-valid rather than any sub module/directory e.g. @emotion/is-prop-valid/dist/whatever.js.

Which I think is a bug with dep-cruiser? It should use the module field for a default import, not one of the browser / exports fields.

The version of @emotion/is-prop-valid is 0.7.3 (as in below link) in my project, and I assume wouldn't be an issue with newer versions, but as it's dependency of one of my dependencies I'm not sure I can fix it myself.

emotion-js/emotion@515f254#diff-ea92ceb68d9de797ff6ccd926051ee30fd069382b1d5d0989626bc25582182deR2

from dependency-cruiser.

After further investigation this is not limited to @emotion/is-prop-valid.

Any package that specifies a browser field (as an object) does not resolve default imports (unless presumably defined as . in the browser field).

from dependency-cruiser.

Here's the spec for the browser field when used as an object in case it helps: https://github.com/defunctzombie/package-browser-field-spec#replace-specific-files---advanced

Not quite the same as an exports field.

from dependency-cruiser.

@emotion/is-prop-valid also defines a browser field, and my new config seems to prefer resolving the browser field over the module field, even though the import is of @emotion/is-prop-valid rather than any sub module/directory e.g. @emotion/is-prop-valid/dist/whatever.js.

It's what I'd indeed expect would happen - with the exportsFields we tell the resolver in which order to evaluate fields in package.json. So if browser is at the start it'll try to interpret that which is in the browser field as an exports field first. And that clearly ends up in tears as you've found.

The browser predates the exports field. The exports field solves the same problem in a more generic fashion. This is likely why @emotion/is-prop-valid@latest has migrated to exports as well. I would expect that if the browser field is a simple string, putting browser in front of the mainFields array would work. It might work for browser-field-as-an-object as well, but this depends on the implementation of the resolver dependency-cruiser currently uses (enhanced-resolve). According to this webpack issue which points to webpack's resolver's [mainFields](https://webpack.js.org/configuration/resolve/#resolvemainfields documentation it should work as expected.

I'd love to have tried this before posting this, but it's late and I'm too tired
to do anything coherent with code a.t.m.

Another approach that might be better cut for your use case

Your initial query was to resolve e.g. const { thing } = require("./some-module") to "./some-module.browser.js" if it exists and to "./some-module.js" if it didn't (check?).

That is something that can be accomplished with the extensions option. Normally you'd use this for extensions like .ts, .cts, .d.ts, .js, .cjs, .jsx etc - but it works for other things as well. When the resolver gets the the module name, it'd plonk the first extension from that array the name and do an fs.access(Sync). If it exists it's done. If it doesn't it'll continue down the array. So

  // ...
  enhancedResolveOptions: {
    extensions: [".browser.js", ".js"] // add your own list of extensions you have in use to this array
  }
  // ...

might be the only thing you need.

from dependency-cruiser.

I'd rather avoid the need for me to manually manipulate the imports (I'm already doing a lot of that), and have the browser files resolved from the browser field.

I've tried with browser in mainFields and exportsFields, and .browser.js in the extensions, but all of them have other adverse effects.

Presumably some config is required for enhanced-resolve to resolve the browser field?
Do you believe dep-cruiser should already be resolving the browser field?

If so, I can put together a small replication to demonstrate the issue.

from dependency-cruiser.

Update: discovered that this needs to be manually configured with the aliasFields property

import fs from 'node:fs';

import { cruise } from 'dependency-cruiser';
import enhancedResolve from 'enhanced-resolve';

const DEFAULT_CACHE_DURATION = 4000;

const dependencies = await cruise(
  pathnames,
  {
    baseDir: process.cwd(),
    builtInModules: {
      override: [],
      add: [],
    },
    enhancedResolveOptions: {
      mainFields: ['module', 'main'],
      exportsFields: ['exports'],
      conditionNames: ['import', 'require', 'default'],
    },
  },
  {
    fileSystem: new enhancedResolve.CachedInputFileSystem(
      fs,
      DEFAULT_CACHE_DURATION
    ),
    aliasFields: ['browser'], //  this bit
    resolveDeprecations: true,
  }
);

Have a few questions related to this:

Why are resolveOptions specified separately from enhancedResolveOptions?
Is there a way to define this without the need to specify the fileSystem and resolveDeprecations?

It would be nice if these had default values so I don't have to manually reference enhanced-resolve etc.

If not, could dep-cruiser expose the DEFAULT_CACHE_DURATION so I don't have to redefine this?

from dependency-cruiser.

Hi @JakeSidSmith - oh wow! Thanks for the research. It'd indeed make sense to add this 'aliasFields' property by default to what dependency-cruiser passes to enhanced-resolve or - in the least - provide an easy way to pass it.

On your questions

Why are resolveOptions specified separately from enhancedResolveOptions?
Good question.

• resolveOptionsreflects the (expanded) webpack.config.js you can pass in the .dependency-cruiser.js config (or as a cli parameter)
• enhancedResolveOptions is the object you can pass in .dependency-cruiser.js for when you don't use webpack, don't want to have a webpack config but still need to tweak parameters in enhanced resolve.

In hindsight in the main function these two could've been combined when we'd design it from the ground up again, possibly moving the logic to combine the two to the cli.

Is there a way to define this without the need to specify the fileSystem and resolveDeprecations?

• On the cli passing a webpack config with the adapted resolve parameters would work.
• Likewise, with the API passing it in the parameter that reflects the webpack config should work.

from dependency-cruiser.

Would be great to see this included with the enhancedResolveOptions. 🥳

Likewise, with the API passing it in the parameter that reflects the webpack config should work.

I don't believe the types currently allow it to be passed anywhere but the resolveOptions.

from dependency-cruiser.

I've checked the typings on the main functions, and it seems they're ok, but I can see where the confusion might stem from as there are three spots to enter things to pass to enhanced resolve ...

export function cruise(
  pFileAndDirectoryArray: string[],
  pCruiseOptions?: ICruiseOptions,
  pResolveOptions?: IResolveOptions,
  pTranspileOptions?: ITranspileOptions
): Promise<IReporterOutput>;

Two in ICruiseOptions:

interface ICruiseOptions {
  // ...
  /**
   * Webpack configuration to use to get resolve options from
   * This is the option to point to a webpack.conf.js (fileName
   * attribute) and pass arguments and an object of webpack
   * env variables. All webpack options (including all those of
   * enhanced-resolve) can be passed in that file.
   */
  webpackConfig?: IWebpackConfig;
  // ....
  /**
   * Options used in module resolution that for dependency-cruiser's
   * use cannot go in a webpack config.
   * E.g. when there isn't/ you don't want to have a webpack.conf.js
   * but still need to pass things onto it.  
   */
  enhancedResolveOptions?: {
  }
}

And one separately (pResolveOptions). The cli takes the ICruiseOptions.webpackConfig, expands them into an object and passes them into that guy. If one uses the API they can either put the enhanced-resolve config they want directly in there or use the function in ./config-utl/extract-webpack-resolve-config to do the expansion of a regular webpack.conf.js:

import type { ResolveOptions, CachedInputFileSystem } from "enhanced-resolve";

/**
 * an object with options to pass to the resolver
 * see https://github.com/webpack/enhanced-resolve#resolver-options
 * for a complete list. Extended with some sauce of our own for practical
 * purposes
 */
interface IResolveOptions extends ResolveOptions {
  /**
   *
   * Without bustTheCache (or with the value `false`) the resolver
   * is initialized only once per session. If the attribute
   * equals `true` the resolver is initialized on each call
   * (which is slower, but might is useful in some situations,
   * like in executing unit tests that verify if different
   * passed options yield different results))
   */
  bustTheCache?: boolean;
  /**
   * We're exclusively using CachedInputFileSystem, hence the
   * rude override
   */
  fileSystem: CachedInputFileSystem;

  /**
   * If true also tries to find out whether an external dependency has
   * been deprecated. Flagged because that's a relatively expensive
   * operation. Typically there's no need to set it as dependency-cruiser
   * will derive this from the rule set (if there's at least one rule
   * looking for deprecations it flips this flag to true)
   */
  resolveDeprecations: boolean;
}

from dependency-cruiser.

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

from dependency-cruiser.