Rxjs: Issues with symbol-observable and AMD

[kitsonk]

RxJS version:

@reactivex/[email protected]

Code to reproduce:

Just loading the AMD modules (@reactivex/rxjs/dist/amd) no longer works, reporting Uncaught Error: Failed to load module symbol-observable from ../../../symbol-observable.js (parent: rxjs/Rx) from the AMD loader.

The AMD loader can be configured to understand where the symbol-observable package is, but then there are two additional problems. The default for AMD package resolution is main.js instead of index.js and they of course don't introspect the package.json when running in a browser. That of course can be configured as well, but then the AMD loader tries to load a CJS module in a browser and of course there is no CJS environment in a browser.

Also, I think it is dangerous/not a good idea to have external 3rd party runtime dependencies for RxJS, ones that are only available as CJS npm packages. I get the concept that there is a desire not to polyfill the whole of Symbol, but since part of RxJS purpose is to polyfill ES Observables, why is not the Symbol.observable polyfill not self contained within the rxjs?

Prior to beta.7 we could use RxJS in our projects with a single line package configuration, much like we would with any external runtime package. Is RxJS actually planning on support AMD in the future?

Expected behavior:

You can load the AMD version of RxJS without errors.

Actual behavior:

Errors as above.

[benlesh]

If symbol-observable had a main.js, that was AMD, would that solve the issue?

[kitsonk]

It would. It would require an extra package configuration, but it wouldn't be the worst thing in the world. Like with any external run-time package though, there are other considerations (like with npm@3 flattening dependencies versus npm@2 not) though most tooling solutions handle that.

While I can't argue it won't work, this is just the first external run-time dependency for RxJS 5. It was nice not having to worry about any before. Even if I were to polyfill it myself, I would still have to have symbol-observable resolved and loaded to get RxJS to load to find out it didn't need it. (I can of course maybe blame TC39 for not addressing conditional imports in the spec 😄)

[benlesh]

cc @jayphelps I think you have more knowledge about AMD than I do. Can you give me a judgement call here?

[jayphelps]

@kitsonk is your build pipeline expecting that the AMD build of symbol-observable be defined anonymously? AMD users in the past have been pretty divided on this, not sure if this ever was settled.

e.g. assuming a file path node_modules/symbol-observable/dist/symbol-observable.js, the contents of which:

// anonymous
define([], factory);

vs. 

// not anonymous
define('symbol-observable', [], factory);

[kitsonk]

Settled, possibly not, but passing an ID is a particularly cruel and unusual punishment. There is seldom benefit in setting it in individual modules and tend to only be used in bundles.

Although it maybe overkill for this, the UMD format generated by TypeScript would work for CJS and AMD.

[jayphelps]

@kitsonk clarify a few things plz:

• You want node_modules/symbol-observable/dist/symbol-observable.js contents to be define([], factory)?
• Are you saying the rxjs UMD version works for you as-is?

[kitsonk]

Sorry to be obtuse, but what I am saying is that @reactivex/[email protected] of /dist/amd/Rx.js worked with an AMD loader and that @reactivex/[email protected] doesn't. I was able to trace that down to:

• RxJS was trying to loaded an external package of which there was no loader configuration for
• That package was not in the AMD format

I assumed RxJS 5 wishes to support AMD (or why distribute it in that format?) and I have suggested a couple ways to fix it. First would be not to relay upon any external runtime packages and have all the modules that need to be loaded contained within the package, written in TypeScript and emitted in the distribution formats provided up to this point. I am assuming that is not how the project wishes to fix the issue.

Therefore, the second option could be to provide the external dependency in a format that is compatible with RxJS. All the modules in RxJS are TypeScript Modules transpiled to AMD, CJS, ES6 and a global distribution. Therefore it would be logical that the external dependency be in the same formats?

The current distributed format of RxJS are anonymous AMD modules, as is the norm for any transpiled modules for TypeScript. For example, /dist/amd/Operator.js:

define(["require", "exports", './Subscriber'], function (require, exports, Subscriber_1) {
    "use strict";
    var Operator = (function () {
        function Operator() {
        }
        Operator.prototype.call = function (subscriber, source) {
            return source._subscribe(new Subscriber_1.Subscriber(subscriber));
        };
        return Operator;
    }());
    exports.Operator = Operator;
});
//# sourceMappingURL=Operator.js.map

Also, to be helpful, I pointed out that, while RxJS distributions don't include it at the moment, the UMD format provided by TypeScript would also work as distribution format for the external dependency, therefore potentially reducing the number for formats that need to be in the external dependency. Logically if someone were to write the external dependency in TypeScript and compile its output to the formats supported by RxJS that would probably work best for everyone (and not just my specific use case).

[jayphelps]

Thanks @kitsonk. That was why I was clarifying, I believe we'll start distributing symbol-observable as UMD (w/ anon AMD).

[kitsonk]

Any chance that this will get fixed?

[jayphelps]

Certainly a chance. Unfortunately time is limited so we have to prioritize what we can. If you have the cycles, we'd love to have your contributions.

[kitsonk]

@jayphelps hopefully blesh/symbol-observable#20 helps.

[benlesh]

I think that blesh/symbol-observable#20 is the real fix for this. I'm going to close this issue for the time being.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.