nxext / nx-extensions

Nx Extensions for Stencil, Svelte, SolidJS, Preact, Ionic, and Capacitor
MIT License
466 stars 102 forks source link

`UNRESOLVED_IMPORT` warning when importing libs from the same Nx workspace #52

Open hnipps opened 4 years ago

hnipps commented 4 years ago

I have a Nx workspace with 1 app and 6 libs (4 of which were generated using @nxext/stencil):

MainLib is dependent on LibA and LibB.

When building MainLib I get this error for both LibA and LibB:

Bundling Warning UNRESOLVED_IMPORT
'@bv0012/lib-a' is imported by
../../../libs/main-lib/src/theme/component-map.ts, but could
not be resolved – treating it as an external dependency

There are no inline errors in component-map.ts and my tsconfig.json paths property looks like this:

"paths": {
      "@bv0012/main-lib": ["libs/main-lib/src/components.d.ts"],
      "@bv0012/main-lib/*": ["libs/main-lib/src/*"],
      "@bv0012/main-lib/loader": ["dist/libs/main-lib/loader"],
      "@bv0012/main-lib/react": [
        "dist/libs/generated/main-lib/react/src/components.ts"
      ],
      "@bv0012/types": ["libs/types/src/index.d.ts"],
      "@bv0012/utils": ["libs/utils/src/index.ts"],
      "@bv0012/lib-a": ["dist/libs/lib-a"],
      "@bv0012/lib-b": ["dist/libs/lib-b"],
      "@bv0012/lib-c": ["dist/libs/lib-c"]
    }

I'm pointing the lib paths to dist/ because they're publishable.

I feel like this is a node-resolve issue but I haven't had any success debugging it so far.

hnipps commented 4 years ago

My current workaround is to npm link each libs dist directory, then npm link @bv0012/lib-a in the root directory and remove the paths for those libs from tsconfig.json.

hnipps commented 4 years ago

I also found out that even though lib-a and lib-b are not bundled with main-lib, the app that depends on main-lib is able to resolve the dependencies correctly without using npm link.

hnipps commented 4 years ago

Here's a minimal reproduction.

Run nx build lib-a followed by nx build main-lib to see the issue.

luchsamapparat commented 4 years ago

It seems that path mappings are not supported out of the box by the stencil compiler which is afaik using rollup under the hood. There are dedicated Rollup plugins for that issue:

https://github.com/bitshiftza/rollup-plugin-ts-paths https://github.com/simonhaenisch/rollup-plugin-typescript-paths https://github.com/Mutefish0/rollup-plugin-typescript-path-mapping

However, in context of Nx we have the additional challenge, that the app is built with apps/my-app/tsconfig.json but the path mappings are configured in the root tsconfig.json (which the app-specific tsconfig.json extends).

If the Rollup plugin uses the root config, the root path does not match the app path and the module resolution fails. If the Rollup plugin uses the app config, the plugins would have to resolve the extends reference to know about the path mappings in the root config, which none of the plugins seem to be capable of.

My goal is to get it running with the standard setup, but if you're open to managing the mappings yourself, if may work if you pass the path mappings directly to the plugin:

https://github.com/Mutefish0/rollup-plugin-typescript-path-mapping/blob/master/index.js#L22

luchsamapparat commented 4 years ago

I spent the afternoon on a PR for rollup-plugin-typescript-paths and got the extends part working (simonhaenisch/rollup-plugin-typescript-paths#5). However, I noticed too late, that the plugin runs after TypeScript compilation:

It assumes that your Typescript code has already been transpiled before being rolled up

Which means that path mappings that reference the index.ts file of a lib won't work (as they are currently generated by the @nxext/stencil plugin):

    "paths": {
      "@my-org/my-lib": [
        "libs/my-lib/src/index.ts"
      ],
      "@my-org/my-lib/loader": [
        "dist/libs/my-lib/loader"
      ]
    }

Also, I'm not even sure if a rollup plugin is the actual solution for our problem, as this comment in the Stencil issues suggests, that the Stencil compiler should be able to handle path mappings on its own 🤔 Maybe this is a Windows issue?

EDIT: I found the PR that adds path mapping support ionic-team/stencil#1584

hnipps commented 4 years ago

@luchsamapparat thank you for sharing your investigation!

FWIW I'm on Mac OS.

I found that the changes introduced in the PR you mentioned were subsequently removed in this PR which has a lot of changes but very little context and has something to do with removing the legacy compiler.

I don't know if they were incorporated into another file because it's a large PR.

luchsamapparat commented 4 years ago

I did a quick check yesterday and created a new project with npm init stencil. I added "@foo": "src/some-file.ts" as path mapping to the generated tsconfig and then added an import for @foo to the generated component - and it worked 🤔

DominikPieper commented 4 years ago

@luchsamapparat thanks for your help.

Sorry for the late response, I'm at holidays right now. I'll take a look at soon as I can

hnipps commented 4 years ago

@DominikPieper no problem, enjoy your holiday :)

@luchsamapparat I'm not seeing an issue with paths like your example. It's when the path is pointing to a build dir, e.g. "@bv0012/lib-a": ["dist/libs/lib-a"].

I did the same quick check using npm init stencil but with a path like above and I still see Bundling Warning UNRESOLVED_IMPORT.

I haven't tried using rollup-plugin-typescript-paths yet, it's on my todo list.

luchsamapparat commented 4 years ago

@hnipps Ah, then Stencil probably didn't implement the module resolution, that TypeScript is perfoming, which is quite complex:

https://github.com/microsoft/TypeScript/blob/2c08affa0d0d7fc55f15ab22e0326b93326d21d8/src/compiler/moduleNameResolver.ts#L927 https://github.com/microsoft/TypeScript/blob/2c08affa0d0d7fc55f15ab22e0326b93326d21d8/src/compiler/moduleNameResolver.ts#L1165

When pointing to a path, the compiler not only looks for a index.ts/.js/.d.ts but also reads the main property of a package.json - all depending on how TypeScript is configured...

Maybe it's worth an issue in Stencil...?

EDIT: the problem is probably somewhere here: https://github.com/ionic-team/stencil/blob/d8d70b23979330045751f9b556d660b51400f696/src/compiler/sys/resolve/resolve-module-async.ts#L9

DominikPieper commented 4 years ago

Okay, now I'm back and looked into this. I'll do some experiments as well. The ugly/fastes solution could be using rollup again to bundle the libs. The @nrwl/web plugin does this, cause you can request the dependency tree und bundle up then. I'll try some more with the stencil compiler but maybe that's an workaround

DominikPieper commented 4 years ago

Hey guys, finally I had some time to investigate deeper into this. Right now I try to find a solution for buildable libs. Trying with stencil 2.0 (maybe something fixed in the meantime) if I export the components I want to use in the libraries index.ts instead of the generated components.d.ts file it works ootb. I'll change the default template for not buildable libraries I think, to work here like the angular builders.

DominikPieper commented 3 years ago

Unfortunately, the new version brings in a half solution—there a now the concept of buildable and not buildable libraries like in the Angular plugins. Import from not buildable plugins works well (just typescript doing some work here) but buildable libs, there's not a solution yet :-(

superpezgeek commented 3 years ago

i was able to solve this by updating paths in tsconfig.base.json to this:

"paths": {
  "@cool-app/component-a": ["dist/libs/component-a/dist"],
  "@cool-app/component-b": ["dist/libs/component-b/dist"],
  "@cool-app/component-c": ["dist/libs/component-c/dist"],
  "@package": ["package.json"]
}

and by importing the packages in the global script:

import '@cool-app/component-a';
import '@cool-app/component-b';
import '@cool-app/component-c';

export default async () => {
  /**
   * The code to be executed should be placed within a default function that is
   * exported by the global script. Ensure all of the code in the global script
   * is wrapped in the function() that is exported.
   */
};

it feels wrong, but it works and doesn't appear to break the dependency graph