[PR WELCOME] Dynamic rollup configuration // print descriptive error messages #312
Comments
|
Question, why not have externals in rollup be a function like the below so that all node modules are treated as externals ? @DavidParks8, @dherges any feedback? |
|
I got a "descriptive" error message but I wonder what can I do about it. @dherges I modified locally Thanks a lot for this awesome tool! |
|
Hi @matheo, how did you include |
|
@alan-agius4 Only saw your comment now. Looks good in theory .... will UMD bundles continue to work? What about the large list of rxjs mappings? |
|
@oliveti I found that any library requiring globals should be handled as So, what happened to me is that I didn't declare my library |
|
On a broader discussion: what if... we analyze the TypeScript imports (AST traversal) and set up the rollup configuration to just what your library source code needs?
Regarding rxjs in specific: rxjs is moving to top-level imports ... but it's still likely that users are running different versions of rxjs and use different import syntax'es. |
dherges
changed the title
|
@dherges UMD should continue to work, the only thing that you'd need to include I am not quite sure how it will play with |
|
@alan-agius4 Can you verify that? Do not speculate here. For UMD, we need a mapping from the module ID (the "path" written in the Example: UMD: (function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core'), require('@angular/cdk/overlay'), require('@angular/cdk/portal'), require('rxjs/observable/range'), require('rxjs/operators'), require('rxjs/Observable'), require('rxjs/operator/map'), require('rxjs/add/operator/map'), require('rxjs/add/observable/of'), require('rxjs/symbol/observable')) :
typeof define === 'function' && define.amd ? define(['exports', '@angular/core', '@angular/cdk/overlay', '@angular/cdk/portal', 'rxjs/observable/range', 'rxjs/operators', 'rxjs/Observable', 'rxjs/operator/map', 'rxjs/add/operator/map', 'rxjs/add/observable/of', 'rxjs/symbol/observable'], factory) :
(factory((global.sample = global.sample || {}, global.sample.externals = {}),global.ng.core,global.ng.cdk.overlay,global.ng.cdk.portal,global.Rx.Observable,global.Rx.Observable.prototype,global.Rx,global.Rx.Observable.prototype,global.Rx.Observable.prototype,global.Rx.Observable,global.Rx.Symbol));
}(this, (function (exports,core,overlay,portal,range,operators,Observable,map$1,map$3,of,observable) { |
|
@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 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: rollup/rollup-plugin-node-resolve#110 |
dherges
changed the title
|
@alan-agius4 Looks good to me! Let's implement dynamic rollup configuration like you suggested. Before returning an Maybe like: A pull request here is welcome! |
|
Just a small note when rollup tries to guess the name of an external, it will print out a warning telling you so, so it might be a bit redundant adding an addition warning, that said of course we can customize the message if you prefer that format. Also, some other questions;
|
|
Hm. Obviously, 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 – 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 |
|
Agreed I like it and sounds good to me, than maybe we should name external
to externalNames? As in some cases for UMD you’d need to pass the names
since rollup won’t be able to guess correctly for some libraries.
What do you think?
…On Sun, 10 Dec 2017 at 14:55, David Herges ***@***.***> wrote:
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 ***@***.***/bar'; in TypeScript code – , then I'd like to treat
the third-party as a peerDependency: { ***@***.***/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?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#312 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQv-Wk8FVC48dLv-Rl_77sRWboB750snks5s--K4gaJpZM4QpKJP>
.
|
|
This issue has been automatically locked due to inactivity. |
Type of Issue
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
The text was updated successfully, but these errors were encountered: