Have workspace Resolution functionality along with Node Module Resolution for dependency resolution

[knarwani]

What I need?
Can we please have the workspace resolution functionality as well along with node_module resolution of the dependencies?

Why I need?
I am using rnx-kit for aligning dependencies in my project. I am working on a monorepo and I wanted to use align-deps before installing any dependencies in my project (To run rnx-kit I have created a bundle of it and calling it). For that I need workspace resolution of few of my internal packages which are yet not available in node_modules. So, can we please have the workspace resolution functionality as well along with node_module resolution of the dependencies?

Where to change?
I think this is the place where my functionality can be added.

[tido64]

Hi @knarwani,

Just to make sure I understood you correctly, are you seeing align-deps fail because you want to use presets that are part of the monorepo? Like we do here:

rnx-kit/packages/test-app/package.json

Lines 173 to 177 in cb1712e

	"presets": [
	"microsoft/react-native",
	"@rnx-kit/scripts/align-deps-preset.js"
	],
	"requirements": [

If that's the case, then yes, that would be the place to add that.

Would you be willing to work on this? We can also do it, but I can't promise any dates.

[knarwani]

Yes, I would like to work on this actively. I will create PR once I find the correct solution for this.

[krishna8770]

Hi there,

I wanted to change this file where loadPreset function will change to

function loadPreset(
  preset: string,
  projectRoot: string,
  resolve = require.resolve
): Preset {
  switch (preset) {
    case "microsoft/react-native":
      return reactNativePreset;
    default: {
      const info = loadInfo(preset);
      if (info !== undefined) {
        const returnResolvedPath = path.join(path.dirname(info.packageJsonPath), parsedInfo.path);
        return require(returnResolvedPath);
      }
      else {
        return require(resolve(preset, { paths: [projectRoot] }));
      }
    }
  }
}

and will add this const in config.ts

export const allPackages = getPackageInfos(process.cwd());

I tried this solution in my local and is working fine. Can you have a look at this code and let me know if I can go ahead and create a PR for this change.

Please let me know what your views on this are.

Thanks.

[tido64]

Hi @krishna8770,

I'm not sure what loadInfo is supposed to do in your example, but I think it looks correct. My only concern is about doing something very expensive in this function as it may be called many times, especially in a big monorepo. It would be ideal if we can somehow pass all the workspace packages to this function. We've already done the work here:

rnx-kit/packages/align-deps/src/cli.ts

Line 198 in dfbf5a4

const manifests = await getManifests(packages);

We should be able to plumb this all the way to loadPreset.

[krishna8770]

Yes, the expensiveness of the function was my concern also so, to address that I have exported this variable from config.ts file i.e., export const allPackages = getPackageInfos(process.cwd()); which then will be calculated only once and then passed on to further iterations. This solution is not at all increasing the runtime on my monorepo with approx 400 packages. Let me know if you are fine with this solution being merged.

[tido64]

We should not use global states as we have no control over how this package may be used. Our best bet is to pipe through the getManifests result to loadPreset as I've mentioned in #2457 (comment).

[krishna8770]

I have made the subsequent changes. Can you please let me know how to create PR on this repo as I am not getting option to create new branch with either of my accounts (Microsoft or personal).

[tido64]

I have made the subsequent changes. Can you please let me know how to create PR on this repo as I am not getting option to create new branch with either of my accounts (Microsoft or personal).

Please follow this guide: https://docs.github.com/en/get-started/quickstart/contributing-to-projects

[krishna8770]

Here is the PR for the change. I had to make a lot of changes even in the test files. Can you please review this once (There are few pipeline check issues, please ignore them for now I am working on them)?