fix: generated tsconfig path mapping for publishable libs (when generated in nested folders)

[juristr]

Current Behavior (This is the behavior we have today, before the PR is merged)

Right now the generated path mappings for nested libraries get generated as follows:

"@my-org/platform/core": ["libs/platform/core/src/index.ts"],
"@my-org/platform/common": ["libs/platform/common/src/index.ts"]

The above works fine as long as the library isn't a buildable one. Buildable libraries have their own package.json where the package name would look like @my-org/platform-core. NPM package names cannot have multiple nestings, but rather just <scope>/<package-name>. As a result the Nx generated path mapping would differ from the one when the package gets published, which is a problem.

Expected Behavior (This is the new behavior we can expect after the PR is merged)

The path mappings generated in the tsconfig.json should match the one of the published package.

Issue

#2794

[Hotell]

• this should be added to react plugin as well 👌

[vsavkin]

I like having importPath as an option. I'd add it to all types of libs that we have. We also need to make sure we use the same value in the name of package.json generated.

I'd pull out the npm check form this PR. I feel like the whole publishable thing is a lot trickier, and I'd tackle when we implement a liteweight build builder. Then we would have an actual disctinction between merely buildable and buildable+publishable.

[vsavkin]

I feel uneasy about us adding more functionality into Nx. Publishing is more custom than building. For instance, the issue with import path isn't just that it cannot contain more than 1 slash, often the scope outside of monorepo is different as well.

We could make the PR smaller.

If we use --buildable, everything works as is. If you use --publishable, we require you to provision --importPath. We don't validate anything. The user knows the path they want to use, they know the scope they use in their org, etc..

In this case, we won't have to have a new default.

  let importPath = options.importPath;
  if (!importPath) {
    importPath = options.publishable
      ? `@${getNpmScope(host)}/${projectName}`
      : `@${getNpmScope(host)}/${projectDirectory}`;
  }

If it's publishable, they have to provision the import path. If not, they can still provision it, but we use the current one by default. I know we went back and forth on it. Sorry about it.

What do you think?

[llwt]

Not sure if the implementation here resolves the issue (I haven't had a chance to check) but we do something similar to this in a custom schematic at the moment and it breaks @nrwl/workspace:move and @nrwl/workspace:remove. We had to wrap them with a custom schematic so it doesn't choke on the tsconfig paths changes.

Just figured I'd leave a friendly heads up just in case :)

[juristr]

@llwt oh thank you!! I've been in vacation last week, so I'm gonna wrap this up this week and test the move schematics as well. Thanks a lot!

I'll let you know once this is out. Would be cool if you'd be willing to test it :)

[llwt]

Hey @juristr, sorry for the delayed response, I've been a bit backed up on notifications 😅

I'd love to test things out once ready, just lmk!

[juristr]

@llwt it's in master. so feel free to give it a try :)

[llwt]

Thanks @juristr. The --importPath option works great and we were able to remove our custom impl from our schematics.

@nrwl/workspace:move however still errors with Cannot read property 'map' of undefined when run on a module that was generated using that flag.

Our custom schematic temporarily updates the tsconfig to the directory path before moving, then reverts it:

export default function (schema: Schema): Rule {
  return chain([
    revertPathInTsConfig(schema),
    moveWorkspace(schema),
    updateRootTsConfig(schema),
  ]);
}

Should I open an issue for this?

[juristr]

Hey, yeah please open a new issue for that & reference me there s.t. I can keep track of it.

Thx a lot

[llwt]

Done: #3476

[github-actions]

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.