ng-packagr: [PR WELCOME] Dynamic rollup configuration // print descriptive error messages

Type of Issue

[x] Feature Request

Description

Many people seem to have the same issue with various libraries: they don’t have correct rollup externals.

How To Reproduce

Look at #310, #301, #85, #119, #108, #142, #230, #163, etc…

Expected Behaviour

I suspect that a lot of these issues can be avoided by printing a more helpful error message.

Version Information

ng-packagr: v2.x.y
node: v8.x.y
@angular: v4.x.y
rxjs:
zone.js:

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Comments: 15 (11 by maintainers)

Most upvoted comments

Question, why not have externals in rollup be a function like the below so that all node modules are treated as externals ?

external: id => !(path.isAbsolute(id) || id.startsWith("."));

@DavidParks8, @dherges any feedback?

Hm. Obviously, lib.externals was driven by rollup’s external and global settings. So that’s what it was in the past.

Now, let’s switch perspective to the present. If I, as an author or maintainer of a library, write a library depending on a third-party – import { ... } from '@foo/bar'; in TypeScript code – , then I’d like to treat the third-party as a peerDependency: { "@foo/bar": "..." } and thus a rollup external – in almost every case.

There is one exception that I found and that we use in an enterprisy case: I, as a maintainer, have legacy javascript code that I’d like to embed in the bundles of my library. Why? The legacy may not be on a npm registry, it may be monkey-patched, … that sort of enterprisy things.

Why do we keep writing external when external is the 9-out-of-10 use case? Why don’t we write embedded for things we’d like to embed and treat everything else as external by default?

@alan-agius4 Looks good to me! Let’s implement dynamic rollup configuration like you suggested. Before returning an undefined value, it should print a warning message.

Maybe like: Library "@your/stuff" depends on "@foo/bar". Please consider to add "@foo/bar" to the "lib.externals" section of your package file!

A pull request here is welcome!

@dherges, sorry for speculating So I did some tests and while rollup does try to provide default values for globals when you don’t specify it, For RxJs and angular it’ won’t work one reason being is the scope.

So in order to make it work properly with Umd you’s still need to provide the globals for instances that rollup cannot “guess”.

So you’d still need to do something like;

external: id => !(path.isAbsolute(id) || id.startsWith("."));
globals:  (x: string) => {
      let regMatch;
      if (regMatch = /^\@angular\/(.+)/.exec(x)) {
        return `ng.${regMatch[1]}`.replace("/", ".")
      }

      if (/^rxjs\/[^/]+$/.test(x)) {
        return "Rx";
      }

      if ( /^rxjs\/(add\/)?observable\/[^/]+$/.test(x)) {
        return "Rx.Observable";
      }

      if ( /^rxjs\/(add\/)?scheduler\/[^/]+$/.test(x)) {
        return "Rx.Scheduler";
      }

      if (/^rxjs\/(add\/)?operator\/[^/]+$/.test(x)) {
        return "Rx.Observable.prototype";
      }

      return undefined; // leave it up to rollup to guess the global name

    },

Relates to: https://github.com/rollup/rollup-plugin-node-resolve/issues/110