Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2: workspace name imports not working with rollup #2086

Closed
matthewjh opened this issue Jul 29, 2020 · 5 comments
Closed

v2: workspace name imports not working with rollup #2086

matthewjh opened this issue Jul 29, 2020 · 5 comments

Comments

@matthewjh
Copy link
Contributor

matthewjh commented Jul 29, 2020

Is this a regression?

Yes, the previous version in which this bug was not present was: ....

Yes. Worked in 1.7.0

Description

Hello. Our codebase, which I am migrating to rules_nodejs v2, uses workspace imports heavily. I followed the v2 migration advice in the wiki, but it doesn't work:

https://github.com/bazelbuild/rules_nodejs/wiki

I assume I am doing something wrong, but I don't know what and there's not much to go on.

I can recreate my problem inside of examples/angular:

examples/angular/src/app/app.module.ts

Replace

import {MaterialModule} from '../shared/material/material.module';

with

import {MaterialModule} from 'examples_angular/src/shared/material/material.module';

BUILD.bazel

load("@build_bazel_rules_nodejs//:index.bzl", "pkg_npm")

pkg_npm(
    name = "examples_angular_pkg",
    package_name = "examples_angular",
    deps = ["//src/shared/material"],
)

examples/angular/src/BUILD.bazel

rollup_bundle(
    name = "bundle-es2015",
    config_file = "rollup.config.js",
    entry_points = {
        ":main.prod.ts": "index",
    },
    output_dir = True,
    deps = [
        "//src",
        "//:examples_angular_pkg",
        "@npm//rollup-plugin-commonjs",
        "@npm//rollup-plugin-node-resolve",
    ],
)

Run
npm run serve-prod

Output

INFO: Invocation ID: 3ad2e532-945e-4d50-b375-574457864321
INFO: Analyzed target //src:prodserver (1 packages loaded, 51 targets configured).
INFO: Found 1 target...
ERROR: C:/users/matt/documents/angular-projects/rules_nodejs/examples/angular/src/BUILD.bazel:133:14: Bundling JavaScript src/bundle-es2015 [rollup] failed (Exit 1)

bazel-out/x64_windows-fastbuild/bin/src/main.prod.mjs  bazel-out/x64_windows-fastbuild/bin/src/bundle-es2015...
[!] Error: 'MaterialModule' is not exported by node_modules\examples_angular\src\shared\material\material.module.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
bazel-out\x64_windows-fastbuild\bin\src\app\app.module.mjs (5:9)
3: import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
4: import { StoreModule } from '@ngrx/store';
5: import { MaterialModule } from 'examples_angular/src/shared/material/material.module';
            ^
6: import { AppRoutingModule } from './app-routing.module';
7: import { AppComponent } from './app.component';
Error: 'MaterialModule' is not exported by node_modules\examples_angular\src\shared\material\material.module.js
    at error (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:10149:30)
    at Module.error (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:14132:9)
    at handleMissingExport (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:14051:21)
    at Module.traceVariable (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:14443:17)
    at ModuleScope.findVariable (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:13141:39)
    at ReturnValueScope.findVariable (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:2981:38)
    at ChildScope.findVariable (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:2981:38)
    at Identifier$1.bind (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:9219:40)
    at ArrayExpression.bind (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:7965:31)
    at ArrayExpression.bind (C:\users\matt\_bazel_matt\ceaolnpb\execroot\examples_angular\node_modules\rollup\dist\rollup.js:10721:15)

🐞 bug report

Affected Rule

The issue is caused by the rule: A clear and concise description of the problem...

🔬 Minimal Reproduction

🔥 Exception or Error






🌍 Your Environment

Operating System:

  
Windows 10
  

Output of bazel version:
3.4.1

Rules_nodejs version:
2.0.1

(Please check that you have matching versions between WORKSPACE file and @bazel/* npm packages.)

  

  

Anything else relevant?

@matthewjh
Copy link
Contributor Author

matthewjh commented Jul 29, 2020

I realised this error was caused by rollup being fed UMD outputs from ts_library. To workaround this I declared a new filegroup target, using that as dep to pkg_npm:

filegroup(
    name = "examples_angular_pkg_esm",
    srcs = ["//src/shared/material"],
    output_group = "es6_sources"
)

pkg_npm(
    name = "examples_angular_pkg",
    # Makes this code importable as "foo"
    package_name = "examples_angular",
    deps = ["examples_angular_pkg_esm"],
)

I assume that shouldn't be required, but regardless, there are more problems:

  1. examples_angular_pkg above includes only the src files in //src/shared/material, not any of its transitive dependencies. Consequently, relative imports from inside these files to transitive dependencies fail.

  2. even if verify hermeticity of node_modules directory #1 was not a problem, and all the transitive deps were included in the pkg, the end result would still be wrong. it seems that the way this works means that a file importing import {fn} from "company/fn" will get a different fn object to a sibling file that does import {fn} from "./fn". This doesn't seem right.

For us, and surely many others, workspace-name imports are merely aliases -- cleaner alternatives to relative imports -- to a single ESM tree.

@alexeagle

@alexeagle
Copy link
Collaborator

Dupe of #2083 but thanks for extra info. We are working on it...

@matthewjh
Copy link
Contributor Author

matthewjh commented Jul 29, 2020 via email

@matthewjh
Copy link
Contributor Author

Hey @alexeagle, just wondering what the latest is on this?

@alexeagle
Copy link
Collaborator

let's followup on #2083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants