systemjs / builder

SystemJS build tool
MIT License
465 stars 122 forks source link

Issue bundling RXJS 5 #826

Open jamesbrobb opened 7 years ago

jamesbrobb commented 7 years ago

Versions

systemjs: 0.19.47 systemjs-builder: 0.15.34 typescript: 2.3.0 rxjs: 5.4.2

Issue

I have this combination working in my dev environment - transpiling, loading and passing tests in Karma and loading all files individually in the browser - but am having problems bundling RXJS for my production build.

tsconfig.json

{
    "compilerOptions": {
        "target": "es5",
        "module": "System",
        "sourceMap": false,
        "emitDecoratorMetadata": true,
        "experimentalDecorators": true,
        "removeComments": false,
        "noImplicitAny": false,
        "lib": [
            "dom",
            "es6"
        ],
        "typeRoots": [
            "./node_modules/@types"
        ],
        "baseUrl": ".",
        "paths": {
            "rxjs/*": ["node_modules/rxjs/*"],
            "redux": ["node_modules/redux/index.d.ts"],
            "redux-observable": ["node_modules/redux-observable/index.d.ts"],
            "ng-redux": ["node_modules/ng-redux/index.d.ts"]
        }
    },
    "include": [
        "./src/**/*.ts"
    ]
}

Gulp task code:

var builder = new Builder('./src', 'src/path/to/system.config.build.js');

    builder.bundle('main', outputFilePath, {
            format: 'register',
            minify: true,
            sourceMaps: false
        })
        .then(function(arg) {
            cb();
        })
        .catch(function(ex) {
            console.log(ex);
            new Error(ex);
        });

SystemJs config:

{

    warnings: true,

    transpiler: 'typescript',

    paths: {
        'main': 'path/to/app.ts',

        'angular': './node_modules/angular/angular.js',
        'angular-aria': './node_modules/angular-aria/angular-aria.js',
        'angular-animate': './node_modules/angular-animate/angular-animate.js',
        'angular-cookies': './node_modules/angular-cookies/angular-cookies.js',
        'angular-sanitize': './node_modules/angular-sanitize/angular-sanitize.js',
        'angular-material': './node_modules/angular-material/angular-material.js',
        'angular-touch': './node_modules/angular-touch/angular-touch.js',
        'angular-cache': './node_modules/angular-cache/dist/angular-cache.js',
        'angular-ui-router': './node_modules/angular-ui-router/release/angular-ui-router.js',
        'angular-translate': './node_modules/angular-translate/dist/angular-translate.js',
        'angular-translate-loader-url': './node_modules/angular-translate-loader-url/angular-translate-loader-url.js',

        'rxjs/*': './node_modules/rxjs/*.js',

        'redux': './node_modules/redux/dist/redux.js',
        'redux-observable': './node_modules/redux-observable/dist/redux-observable.js',
        'redux-logger': './node_modules/redux-logger/dist/redux-logger.js',
        'ng-redux':'./node_modules/ng-redux/dist/ng-redux.js'
    },

    map: {
        'typescript': './node_modules/typescript/lib/typescript.js',
    },

    meta: {
        'angular': {
            format: 'global',
            exports: 'angular'
        },
        'angular-aria': {
            format: 'global',
            deps: ['angular']
        },
        'angular-animate': {
            format: 'global',
            deps: ['angular']
        },
        'angular-sanitize': {
            format: 'global',
            deps: ['angular']
        },
        'angular-material': {
            format: 'global',
            deps: ['angular']
        },
        'angular-cookies': {
            format: 'global',
            deps: ['angular']
        },
        'angular-touch': {
            format: 'global',
            deps: ['angular']
        },
        'angular-cache': {
            format: 'global',
            deps: ['angular']
        },
        'angular-ui-router': {
            format: 'global',
            deps: ['angular']
        },
        'angular-translate': {
            format: 'global',
            deps: ['angular']
        },
        'angular-translate-loader-url': {
            format: 'global',
            deps: ['angular-translate']
        },
        'redux-observable': {
            deps: ['redux']
        },
        'redux-logger': {
            deps: ['redux']
        },
        'ng-redux': {
            deps: ['redux', 'angular']
        }
    }
}

With this configuration i get the following error:

 { Error tracing rxjs/add/operator/map at file:///Users/blah/blah/project/node_modules/rxjs/add/operator/map.js
    Loading blah
    Loading blah
    Loading blah
    Loading main
    Error: Unable to calculate canonical name to bundle file:///Users/blah/blah/project/node_modules/rxjs/Observable. Ensure that this module sits within the baseURL or a wildcard path config.
at getCanonicalNamePlain (/Users/blah/blah/project/node_modules/systemjs-builder/lib/utils.js:227:13)
at getCanonicalName (/Users/blah/blah/project/node_modules/systemjs-builder/lib/utils.js:150:19)
at /Users/blah/blah/project/node_modules/systemjs-builder/lib/trace.js:565:36

The offending RXJS code (in this instance at least - the error appears to relate to any rxjs file that is required within one of their files)

"use strict";
var Observable_1 = require('../../Observable');
var map_1 = require('../../operator/map');
Observable_1.Observable.prototype.map = map_1.map;

Attempted fix

I've tried removing the wildcards from the the rxjs path and adding a packages definition, which appears to possibly fix this issue, but unfortunately it results in a completely different error.

So in the config this:

'rxjs/*': './node_modules/rxjs/*.js'

changes to this:

'rxjs/': './node_modules/rxjs/',

plus the addition of:

packages: {
    'rxjs': {
        defaultExtension: 'js'
    }
}

Error after changes

    { SyntaxError: path/to/MyEnum.ts: 'import' and 'export' may appear only with 'sourceType: module' (3:0)
  1 | 
  2 | 
> 3 | export enum MyEnum {
    | ^
  4 |     do_a,
  5 |     do_b
  6 | }
    at Parser.pp$5.raise (/Users/blah/blah/project/node_modules/babylon/lib/index.js:4454:13)
    at Parser.pp$1.parseStatement (/Users/blah/blah/project/node_modules/babylon/lib/index.js:1881:16)
    at Parser.pp$1.parseBlockBody (/Users/blah/blah/project/node_modules/babylon/lib/index.js:2268:21)
    at Parser.pp$1.parseTopLevel (/Users/blah/blah/project/node_modules/babylon/lib/index.js:1778:8)
    at Parser.parse (/Users/blah/blah/project/node_modules/babylon/lib/index.js:1673:17)
    at parse (/Users/blah/blah/project/node_modules/babylon/lib/index.js:7246:37)
    at File.parse (/Users/blah/blah/project/node_modules/babel-core/lib/transformation/file/index.js:517:15)
    at File.parseCode (/Users/blah/blah/project/node_modules/babel-core/lib/transformation/file/index.js:602:20)
    at /Users/blah/blah/project/node_modules/babel-core/lib/transformation/pipeline.js:49:12
    at File.wrap (/Users/blah/blah/project/node_modules/babel-core/lib/transformation/file/index.js:564:16)
    at Pipeline.transform (/Users/blah/blah/project/node_modules/babel-core/lib/transformation/pipeline.js:47:17)
    at Object.exports.compile (/Users/blah/blah/project/node_modules/systemjs-builder/compilers/compiler.js:24:22)
    at Object.exports.compile (/Users/blah/blah/project/node_modules/systemjs-builder/compilers/global.js:14:19)
    at /Users/blah/blah/project/node_modules/systemjs-builder/lib/compile.js:117:43
    at tryCatcher (/Users/blah/blah/project/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/blah/blah/project/node_modules/bluebird/js/release/promise.js:512:31)
  pos: 2,
  loc: Position { line: 3, column: 0 },
  _babel: true,
  codeFrame: '\u001b[0m \u001b[90m 1 | \u001b[39m\n \u001b[90m 2 | \u001b[39m\n\u001b[31m\u001b[1m>\u001b[22m\u001b[39m\u001b[90m 3 | \u001b[39m\u001b[36mexport\u001b[39m \u001b[36menum\u001b[39m \u001b[33mClassActions\u001b[39m {\n \u001b[90m   | \u001b[39m\u001b[31m\u001b[1m^\u001b[22m\u001b[39m\n \u001b[90m 4 | \u001b[39m    do_a\u001b[33m,\u001b[39m\n \u001b[90m 5 | \u001b[39m    do_b\n \u001b[90m 6 | \u001b[39m}\u001b[0m' }
aluanhaddad commented 7 years ago

Firstly, using package config is the right approach.

You need to point rxjs to the subdirectory containing the output. That error comes from the Builder directly trying to parse an esm source file. You could configure the package "format": "esm" but as you can see from the error it is also trying to build directly from a typescript file so it would still fail. Is actually possible to build rxjs directly from the typescript source code using the systemjs builder but you would need to install the typescript plugin for that.

jamesbrobb commented 7 years ago

@aluanhaddad thanks for the response.

It had occurred to me that i could possibly just target and transpile the typescript source instead, but i was unaware of plugin-typescript.

However having now installed and configured it, unfortunately i'm still getting the same 'import' and 'export' may appear only with 'sourceType: module' error.

jamesbrobb commented 7 years ago

You need to point rxjs to the subdirectory containing the output.

Could you please elaborate on what you mean by this, I'm not sure i understand?

jamesbrobb commented 7 years ago

After removing the wildcards from the rxjs path (which fixed the initial Error tracing issue), i wrongly assumed that the second issue was also being caused by rxjs. It turns out that that wasn't the case, it was actually being caused by the enum in the error.

I ended up finding 2 seperate enum definitions that would individually cause this error to occur. Although i'm not sure why this is the case, as there are lots of other enum's within my code base that transpile/build without any issues.

The fix for this issue was the one that @aluanhaddad suggested, but i'd possibly misunderstood. Which was to configure the offending package (the one the enum belonged to, not rxjs) as format: 'esm'

aluanhaddad commented 7 years ago

@jamesbrobb I meant that since

You need to point rxjs to the subdirectory containing the output.

Could you please elaborate on what you mean by this, I'm not sure i understand?

What i meant was that the error

{ SyntaxError: path/to/MyEnum.ts: 'import' and 'export' may appear only with 'sourceType: module' (3:0)
  1 | 
  2 | 
> 3 | export enum MyEnum {
    | ^
  4 |     do_a,
  5 |     do_b
  6 | }

implies that babel is being used to transpile an enum which is typescript syntax. You may need to update the map or packages config to point at the built output, instead of the source code, or you can configure plugin-typescript to load it and transpile it on the fly (this will enable tree shaking incidentally)

jamesbrobb commented 7 years ago

Thanks @aluanhaddad

or you can configure plugin-typescript to load it and transpile it on the fly

But is this something that can be used within a production environment?

jamesbrobb commented 7 years ago

I'm starting to feel like a dog chasing his tail. Every time i fix one thing, it seems to just create another issue.

In order to fix the initial problem, i removed the wildcards from the rxjs paths config.

So:

paths: {
    'rxjs/*': './node_modules/rxjs/src/*.ts' 
}

has become:

paths: {
    'rxjs/': './node_modules/rxjs/src/' 
}

According to the depreciation warnings, this is now the correct way to configure a 'wildcard' path, by just leaving a trailing slash. But in order to get it to correctly target .ts files within that package, am i right in thinking that i also need to add the following to the config, as otherwise it will attempt to load `.js' files instead:

packages: {
    rxjs: {
        defaultExtension: 'ts'
    }
}

But using this format adds .ts onto all of the systemjs register paths. so any other library i use that uses rxjs and imports it without the .ts will fail to find those files in the register.

So whereas when using wildcards (if it had worked) the output would be:

System.register("rxjs/observable/from", ["./FromObservable"], function (exports_1, context_1) {
    ...
}

Without the wildcards and only a trailing slash and defaultExtension: 'ts' it outputs:

System.register("rxjs/observable/from.ts", ["./FromObservable.ts"], function (exports_1, context_1) {
    ...
}

and using plugin-typescript it ends up with only .ts on the register keys, but not the dependencies:

System.register("rxjs/observable/from.ts", ["./FromObservable"], function (exports_1, context_1) {
    ...
}

Am i doing something wrong here?

aluanhaddad commented 7 years ago

It is true that the wildcard configuration is deprecated, but you are using an older version of the builder and an older version SystemJS...

The simplest way to make the app work is to just reference one of the bundled versions of RxJS.

Looking at your configuration again, it seems you are using paths config where you should be using map config.

or you can configure plugin-typescript to load it and transpile it on the fly But is this something that can be used within a production environment?

No, but when you run a build/bundle, the TypeScript will be transpiled. Regardless, this is an advanced scenario.

jamesbrobb commented 7 years ago

The latest version of systemjs-builder doesn't use the latest version of systemjs, it uses 0.19.47. And I'm unable to upgrade to the latest version of systemjs due to another issue that's been introduced relating to rxjs (which I don't have access to on my phone at the moment).

By 'rxjs bundle' do you mean use the entire rxjs library by loading Rx.min.js?

Regardless, why is '.ts' now being added to the registry keys and import paths? Is that actually a conscious, deliberate decision?

On 7 Aug 2017 16:29, "Aluan Haddad" notifications@github.com wrote:

It is true that the wildcard configuration is deprecated, but you are using an older version of the builder and an older version SystemJS...

The simplest way to make the app work is to just reference one of the bundled versions of RxJS.

Looking at your configuration again, it seems you are using paths config where you should be using map config.

or you can configure plugin-typescript to load it and transpile it on the fly But is this something that can be used within a production environment?

No, but when you run a build/bundle, the TypeScript will be transpiled. Regardless, this is an advanced scenario.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/systemjs/builder/issues/826#issuecomment-320696206, or mute the thread https://github.com/notifications/unsubscribe-auth/ABq8tQ0lnu87XTWvKKVRqCCpkW460FOgks5sVy1UgaJpZM4OmrZ7 .

jamesbrobb commented 7 years ago

I've also never been able to get maps to work in a systemjs-builder config, only when using systemjs to load files.

Please can you give me an example of what a maps configuration would look like to fix this issue?

On 7 Aug 2017 16:29, "Aluan Haddad" notifications@github.com wrote:

It is true that the wildcard configuration is deprecated, but you are using an older version of the builder and an older version SystemJS...

The simplest way to make the app work is to just reference one of the bundled versions of RxJS.

Looking at your configuration again, it seems you are using paths config where you should be using map config.

or you can configure plugin-typescript to load it and transpile it on the fly But is this something that can be used within a production environment?

No, but when you run a build/bundle, the TypeScript will be transpiled. Regardless, this is an advanced scenario.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/systemjs/builder/issues/826#issuecomment-320696206, or mute the thread https://github.com/notifications/unsubscribe-auth/ABq8tQ0lnu87XTWvKKVRqCCpkW460FOgks5sVy1UgaJpZM4OmrZ7 .

jamesbrobb commented 7 years ago

No, but when you run a build/bundle, the TypeScript will be transpiled. R egardless, this is an advanced scenario

I'm sorry i'm confused. The problem I'm trying to solve is to build/bundle for a production environment. Are you suggesting that using plugin-typescript to transpile on the fly, is or isn't a possible solution for this specific problem? I have no issues with current systemjs configuration in my dev environment.

On 7 Aug 2017 16:29, "Aluan Haddad" notifications@github.com wrote:

It is true that the wildcard configuration is deprecated, but you are using an older version of the builder and an older version SystemJS...

The simplest way to make the app work is to just reference one of the bundled versions of RxJS.

Looking at your configuration again, it seems you are using paths config where you should be using map config.

or you can configure plugin-typescript to load it and transpile it on the fly But is this something that can be used within a production environment?

No, but when you run a build/bundle, the TypeScript will be transpiled. Regardless, this is an advanced scenario.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/systemjs/builder/issues/826#issuecomment-320696206, or mute the thread https://github.com/notifications/unsubscribe-auth/ABq8tQ0lnu87XTWvKKVRqCCpkW460FOgks5sVy1UgaJpZM4OmrZ7 .

aluanhaddad commented 7 years ago

@jamesbrobb I want to apologize for any confusion my remarks have caused.

No, but when you run a build/bundle, the TypeScript will be transpiled. R egardless, this is an advanced scenario

I'm sorry i'm confused. The problem I'm trying to solve is to build/bundle for a production environment.

What I mean is that targeting the TypeScript source of a third party package, compiling them from source is, whether for development or for production and whether bundling or not bundling, an advanced scenario. Here are two example use cases:

A. I am building for production and wish to reduce code size by using plugin-typescript, together with the systemjs-builder's rollup support, to optimize the size of my bundle.

B. I want to use the source code of RxJS for debugging purposes.