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

fix(moment): use replace plugin to replace moment #331

Merged
merged 1 commit into from Sep 22, 2017
Merged

fix(moment): use replace plugin to replace moment #331

merged 1 commit into from Sep 22, 2017

Conversation

LinboLen
Copy link
Contributor

@LinboLen LinboLen commented Sep 19, 2017

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #315 #85 #169 #240

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@trotyl
Copy link
Contributor

trotyl commented Sep 19, 2017

We don't provide sourcemap at all, what is it going to fix?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 40.774% when pulling d9a1284 on gradii:fix-rollup-sourcemap into 6e4794b on NG-ZORRO:master.

@LinboLen
Copy link
Contributor Author

LinboLen commented Sep 19, 2017

error occurs in my project. if there are no correct sourcemap file. rollup can't bundle file

Before

import sourcemaps from 'rollup-plugin-sourcemaps';
import replace from 'rollup-plugin-replace';

function replaceMoment() {
  return {
    name: 'replace-moment-import',
    transform: code =>
            ({
              code: code.replace('import * as moment', 'import moment'),
            })
  }
}
export default {plugins: [replaceMoment(), sourcemaps()]};

can't generate bundle file from esm5/index.js

image

sourcemap file is incorrectly

image

After

import sourcemaps from 'rollup-plugin-sourcemaps';
import replace from 'rollup-plugin-replace';

export default { plugins: [replace({ "import * as moment": "import moment" }),sourcemaps()] };

image


issue occur with rollup command

node_modules/.bin/rollup -c packages/time-picker/rollup.config.js

rollup.config.js

import resolve from 'rollup-plugin-node-resolve';
import sourcemaps from 'rollup-plugin-sourcemaps';

const globals = {

  '@angular/animations':                  'ng.animations',
  '@angular/core':                        'ng.core',
  '@angular/common':                      'ng.common',
  '@angular/forms':                       'ng.forms',
  '@angular/http':                        'ng.http',
  '@angular/router':                      'ng.router',
  '@angular/platform-browser':            'ng.platformBrowser',
  '@angular/platform-server':             'ng.platformServer',
  '@angular/platform-browser-dynamic':    'ng.platformBrowserDynamic',
  '@angular/platform-browser/animations': 'ng.platformBrowser.animations',
  '@angular/core/testing':                'ng.core.testing',
  '@angular/common/testing':              'ng.common.testing',
  '@angular/http/testing':                'ng.http.testing',

  '@angular/cdk':         'ng.cdk',
  '@angular/cdk/testing': 'ng.cdk.testing',

  'rxjs/BehaviorSubject':               'Rx',
  'rxjs/Observable':                    'Rx',
  'rxjs/Subject':                       'Rx',
  'rxjs/Subscription':                  'Rx',
  'rxjs/Observer':                      'Rx',
  'rxjs/Scheduler':                     'Rx',
  'rxjs/observable/combineLatest':      'Rx.Observable',
  'rxjs/observable/forkJoin':           'Rx.Observable',
  'rxjs/observable/fromEvent':          'Rx.Observable',
  'rxjs/observable/merge':              'Rx.Observable',
  'rxjs/observable/of':                 'Rx.Observable',
  'rxjs/observable/throw':              'Rx.Observable',
  'rxjs/observable/defer':              'Rx.Observable',
  'rxjs/operator/auditTime':            'Rx.Observable.prototype',
  'rxjs/operator/catch':                'Rx.Observable.prototype',
  'rxjs/operator/debounceTime':         'Rx.Observable.prototype',
  'rxjs/operator/do':                   'Rx.Observable.prototype',
  'rxjs/operator/filter':               'Rx.Observable.prototype',
  'rxjs/operator/finally':              'Rx.Observable.prototype',
  'rxjs/operator/first':                'Rx.Observable.prototype',
  'rxjs/operator/let':                  'Rx.Observable.prototype',
  'rxjs/operator/map':                  'Rx.Observable.prototype',
  'rxjs/operator/share':                'Rx.Observable.prototype',
  'rxjs/operator/startWith':            'Rx.Observable.prototype',
  'rxjs/operator/switchMap':            'Rx.Observable.prototype',
  'rxjs/operator/takeUntil':            'Rx.Observable.prototype',
  'rxjs/operator/toPromise':            'Rx.Observable.prototype',
  'rxjs/operator/throttleTime':         'Rx.Observable.prototype',
  'rxjs/operator/distinctUntilChanged': 'Rx.Observable.prototype',

  'rxjs/add/observable/merge':              'Rx.Observable',
  'rxjs/add/observable/fromEvent':          'Rx.Observable',
  'rxjs/add/observable/of':                 'Rx.Observable',
  'rxjs/add/observable/interval':           'Rx.Observable',
  'rxjs/add/operator/startWith':            'Rx.Observable.prototype',
  'rxjs/add/operator/map':                  'Rx.Observable.prototype',
  'rxjs/add/operator/debounceTime':         'Rx.Observable.prototype',
  'rxjs/add/operator/distinctUntilChanged': 'Rx.Observable.prototype',
  'rxjs/add/operator/first':                'Rx.Observable.prototype',
  'rxjs/add/operator/catch':                'Rx.Observable.prototype',
  'rxjs/add/operator/switchMap':            'Rx.Observable.prototype',

  'moment': 'moment'
};

export default {
  input: '../../dist/packages-dist/time-picker/esm5/index.js',
  output: {
    file: '../../dist/packages-dist/time-picker/bundles/time-picker.umd.js',
    format: 'umd',
  },
  exports: 'named',
  name: 'ng-zorro-fork.time-picker',
  plugins: [resolve(), sourcemaps()],
  external: Object.keys(globals),
  globals: globals
};

@trotyl
Copy link
Contributor

trotyl commented Sep 19, 2017

The process for generating esm5 index file is: ngc -> rollup -> tsc, as tsc won't consume the input sourcemap generated by rollup, the final sourcemap would be wrong anyway.

Actually the input sourcemap feature for TypeScript has not landed yet: https://github.com/Microsoft/TypeScript/issues/13944.

What's the benefit for having a wrong soucemap over not having it?

@trotyl
Copy link
Contributor

trotyl commented Sep 19, 2017

Another question, would rollup consume the input sourcemap from ngc? The input files for rollup are already generated files.

@trotyl
Copy link
Contributor

trotyl commented Sep 19, 2017

This PR is fine except title not being helpful.

@LinboLen LinboLen changed the title fix moment rollup compile with incorrect sourcemap use replace plugin to replace moment Sep 19, 2017
@LinboLen LinboLen changed the title use replace plugin to replace moment fix(moment): use replace plugin to replace moment Sep 21, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 40.791% when pulling fa57403 on gradii:fix-rollup-sourcemap into 525edb8 on NG-ZORRO:master.

@vthinkxie vthinkxie merged commit aec9f83 into NG-ZORRO:master Sep 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants