Rules_nodejs: concatjs no longer loading directory/index files correctly

[jbedard]

Affected Rule

ts_devserver
karma_web_test_suite
(anything using concatjs)

Is this a regression?

Yes, somewhere between 0.36.1 and 0.38.3

Description

When depending on a directory containing an index file such as import ".../subdir" the AMD module is defined as .../subdir/index but requested as .../subdir.js causing an http request and 404.

🔬 Minimal Reproduction

https://github.com/jbedard/rules_nodejs/commit/d288dc2ea3cd23bd37164580bba4424906184227

[alexeagle]

@jbedard if you get some time to track it down further that would be helpful

[jbedard]

Seems like this has something to do with ivy more then anything, or at least that's the change that started triggering this for me.

I have a lot of .../dir/index.ts files which normally export the "public" things from that directory and just to make import statements nicer, so imports will be from ".../dir". Normally this gets converted to .../dir/index before the final AMD/UMD for concatjs, but it seems like ivy generates an extra require(".../dir") for params of the setClassMetadata. This extra require(".../dir") does not get converted to the .../dir/index version. So at runtime the .../dir AMD module is not defined.

[jbedard]

Seems like I've run into a similar issue, but this time involving ng_module(module_name) instead of imports involving index.ts files. But they are both related to concatjs and import paths...

I'm still having trouble writing isolated tests for either of these cases, but for this one my situation is something such as:

• a folder with ng_module(module_name=@foo/bar) with an index.ts exporting stuff from within the folder
• various locations that import "@foo/bar"
• a separate ng_module that import files from the foo/bar folder with relative paths (so those relative paths do not use the module_name).

I guess it sort of makes sense that the defined AMD module name (from module_name) is different from the imported one (the relative path), so the error might actually be valid, and easy for me to fix. But I'm more curious why this did not exist in 0.36.1 but does in 0.40.0...

@gregmagolan can you think of anything that could have changed with all the recent js-module changes that might effect this? Anything related to AMD module names/imports?

[jbedard]

Here's another attempt at reproducing it while using module_name: https://github.com/jbedard/rules_nodejs/commit/b72792a40a5c748103a42858c59dbe87c68f88f9

Note the various methods of importing subdirtest. But the resulting concatjs file changes all those to only @special/subdirindex and @special/subdirindex/lib. Where in my repo it doesn't change them like that.

The only changes I can think of that may have caused this is the recent js-module changes, or how Ivy builds the generated import statements?

[zaucy]

We were using the angular view engine with bazel but recently _attempted_ a switch to ivy. We ran into something similar to this. _Please let me know if you believe this isn't related and I'll make another issue_

I modified the rules_nodejs angular examples to showcase this breaking when switching to ivy. I removed the hello world component as a route and added it as a child component of todo.

Comparison:
https://github.com/bazelbuild/rules_nodejs/compare/master...ChangeGamers:IvyIndexImportBug

Branch:
https://github.com/ChangeGamers/rules_nodejs/tree/IvyIndexImportBug

I can reproduce it from the branch above with the following:

This has no runtime error

cd examples/angular_view_engine
bazel run src:devserver

This has a runtime error

cd examples/angular
bazel run src:devserver

Here's the error:

core.umd.js:3996 ERROR Error: Uncaught (in promise): Error: Script error for "examples_angular/src/app/hello-world", needed by: examples_angular/src/app/todos/todos.component

Note:

If I switch the import to examples_angular/src/app/hello-world/index, ../hello-world or ../hello-world/index the runtime error is gone.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

[jbedard]

FWIW I'm still seeing this and ran into it just last week.