angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.76k stars 11.97k forks source link

A polyfill for Intl should be included by default #1675

Closed johnchristopherjones closed 8 years ago

johnchristopherjones commented 8 years ago

On Safari 9.1.1 for OS X (macOS) El Capitan and iOS 9.3.3, Angular 2 raises an uncaught exception when any built-in formatting pipe is used. To work in Safari, a polyfill for Intl must be included.

This bug can be reproduced by opening the following plnkr in Safari: http://plnkr.co/edit/ETQGYstBK8MF8YVF78Kj?p=preview

This bug can also be reproduced in angular-cli, version 1.0.0-beta.11-webpack.2:

  1. Install Angular 2.0.0-rc5 using angular-cli:

    $ npm uninstall -g angular-cli && npm cache clean && npm install -g angular-cli@webpack
    $ ng new intl-bug
  2. Add the following to a template in app.component.html:

    <p>{{0.5 | percent:'1.1-1'}}</p>
  3. An uncaught exception will be written to the console. The template code will not appear, anything in the template after the above segment will fail to render, no further events or change detection will be handled, … basically everything breaks right there.

This bug was filed at angular/angular#10764, but closed recommending the polyfill. Since Angular 2's built-in formatting filters fail on Safari without the polyfill, it should probably be a best practice of angular-cli to include the Intl polyfill by default.

filipesilva commented 8 years ago

We're looking at ways to make the polyfills list more explicit so people can pick which ones they want for their project. It's on the radar.

kylecordes commented 8 years ago

My usual boring suggestion, but I think worth repeating:

filipesilva commented 8 years ago

I think that's a good suggestion tbh.

xmlking commented 8 years ago

I have this as workaround in index.html Which loads based on browser

<!-- for Safari -->
  <script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=Intl.~locale.en"></script>
chompy18 commented 8 years ago

Any news regarding this? I have installed the intl npm package and imported it in polyfills.ts, still getting errors on PhantomJS (on specs that pass on Chrome):

PhantomJS 2.1.1 (Mac OS X 0.0.0) Data table component Test a single telemetry event should display time in 'sort' format FAILED
        Error: Error in ./DataTableComponent class DataTableComponent - inline template:9:37 caused by: Invalid regular expression: unmatched parentheses in src/main/js/test.ts (line 73563)
        RegExp@[native code]
        RegExp@webpack:///Users/sninio/dev/csp-ui/~/core-js/modules/es6.regexp.constructor.js:26:0 <- src/main/js/test.ts:92090:19
        webpack:///Users/sninio/dev/csp-ui/~/intl/lib/core.js:590:0 <- src/main/js/test.ts:93307:30
        InitializeNumberFormat@webpack:///Users/sninio/dev/csp-ui/~/intl/lib/core.js:2077:0 <- src/main/js/test.ts:94794:18
        NumberFormatConstructor@webpack:///Users/sninio/dev/csp-ui/~/intl/lib/core.js:1838:0 <- src/main/js/test.ts:94555:34
        CreateDateTimeParts@webpack:///Users/sninio/dev/csp-ui/~/intl/lib/core.js:3898:0 <- src/main/js/test.ts:96615:35
        FormatDateTime@webpack:///Users/sninio/dev/csp-ui/~/intl/lib/core.js:4079:0 <- src/main/js/test.ts:96796:36
        F@webpack:///Users/sninio/dev/csp-ui/~/intl/lib/core.js:3848:0 <- src/main/js/test.ts:96565:34
        F@[native code]
        intlDateFormat@webpack:///Users/sninio/dev/csp-ui/~/@angular/common/src/facade/intl.js:117:0 <- src/main/js/test.ts:53565:59
        webpack:///Users/sninio/dev/csp-ui/~/@angular/common/src/facade/intl.js:148:36 <- src/main/js/test.ts:53596:59
        dateFormatter@webpack:///Users/sninio/dev/csp-ui/~/@angular/common/src/facade/intl.js:157:0 <- src/main/js/test.ts:53605:39
        format@webpack:///Users/sninio/dev/csp-ui/~/@angular/common/src/facade/intl.js:192:0 <- src/main/js/test.ts:53640:29
        transform@webpack:///Users/sninio/dev/csp-ui/~/@angular/common/src/pipes/date_pipe.js:92:0 <- src/main/js/test.ts:70479:90
        transform@webpack:///Users/sninio/dev/csp-ui/src/main/js/app/pages/+platform/events/data-table/data-table.pipe.ts:9:4418 <- src/main/js/test.ts:52698:5787
        [native code]
        webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view_utils.js:158:0 <- src/main/js/test.ts:28876:24
        detectChangesInternal
        detecs@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:271:0 <- src/main/js/test.ts:73441:35
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:376:0 <- src/main/js/test.ts:73546:48
        detectContentChildrenChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:289:0 <- src/main/js/test.ts:73459:32
        detectChangesInternal
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:271:0 <- src/main/js/test.ts:73441:35
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:376:0 <- src/main/js/test.ts:73546:48
        detectContentChildrenChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:289:0 <- src/main/js/test.ts:73459:32
        detectChangesInternal
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:271:0 <- src/main/js/test.ts:73441:35
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:376:0 <- src/main/js/test.ts:73546:48
        detectViewChildrenChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:297:0 <- src/main/js/test.ts:73467:32
        detectChangesInternal
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:271:0 <- src/main/js/test.ts:73441:35
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view.js:376:0 <- src/main/js/test.ts:73546:48
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/linker/view_ref.js:130:0 <- src/main/js/test.ts:56741:33
        _tick@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/bundles/core-testing.umd.js:235:0 <- src/main/js/test.ts:2368:49
        webpack:///Users/sninio/dev/csp-ui/~/@angular/core/bundles/core-testing.umd.js:249:45 <- src/main/js/test.ts:2382:58
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:203:0 <- src/main/js/test.ts:103279:33
        onInvoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/proxy.js:72:0 <- src/main/js/test.ts:68796:45
        invoke@web/Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:202:0 <- src/main/js/test.ts:103278:42
        onInvoke@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/zone/ng_zone_impl.js:43:0 <- src/main/js/test.ts:74147:43
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:202:0 <- src/main/js/test.ts:103278:42
        run@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:96:0 <- src/main/js/test.ts:103172:49
        runInner@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/zone/ng_zone_impl.js:70:51 <- src/main/js/test.ts:74174:74
        run@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/src/zone/ng_zone.js:230:42 <- src/main/js/test.ts:46580:74
        detectChanges@webpack:///Users/sninio/dev/csp-ui/~/@angular/core/bundles/core-testing.umd.js:249:0 <- src/main/js/test.ts:2382:32
        webpack:///Users/sninio/dev/csp-ui/src/main/js/app/pages/+platform/events/data-table/data-table.component.spec.ts:72:38 <- src/main/js/test.ts:60783:40
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:203:0 <- src/main/js/test.ts:103279:33
        onInvoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/proxy.js:72:0 <- src/main/js/test.ts:68796:45
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:202:0 <- src/main/js/test.ts:103278:42
        run@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:96:0 <- src/main/js/test.ts:103172:49
        webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/jasmine-patch.js:91:27 <- src/main/js/test.ts:68532:53
        execute@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/jasmine-patch.js:119:0 <- src/main/js/test.ts:68560:46
        invokeTask@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:236:0 <- src/main/js/test.ts:103312:42
        runTask@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:136:0 <- src/main/js/test.ts:103212:57
        drainMicroTaskQueue@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:368:0 <- src/main/js/test.ts:103444:42
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:308:0 <- src/main/js/test.ts:103384:44
        Expected '' to equal '1/1/:00 AM'.
        webpack:///Users/sninio/dev/csp-ui/src/main/js/app/pages/+platform/events/data-table/data-table.component.spec.ts:99:50 <- src/main/js/test.ts:60805:50
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:203:0 <- src/main/js/test.ts:103279:33
        onInvoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/proxy.js:72:0 <- src/main/js/test.ts:68796:45
        invoke@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:202:0 <- src/main/js/test.ts:103278:42
        run@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:96:0 <- src/main/js/test.ts:103172:49
        webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/jasmine-patch.js:91:27 <- src/main/js/test.ts:68532:53
        execute@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/jasmine-patch.js:119:0 <- src/main/js/test.ts:68560:46
        invokeTask@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:236:0 <- src/main/js/test.ts:103312:42
        runTask@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:136:0 <- src/main/js/test.ts:103212:57
        drainMicroTaskQueue@webpack:///Users/sninio/dev/csp-ui/~/zone.js/dist/zone.js:368:0 <- src/main/js/test.ts:103444:42
johnchristopherjones commented 8 years ago

Per cexbrayat on StackOverflow, the current version of angular-cli now exposes src/polyfills.ts, so that one can fix this particular problem by installing intl:

npm install --save intl

then adding the polyfills to polyfills.ts

import 'intl';
import 'intl/locale-data/jsonp/en';

This is a million times better than the situation when I filed this issue. I don't know if this is quite finished since this particular polyfill is still so essential, but I'm much happier about this today.

Meligy commented 8 years ago

@johnchristopherjones are there any Angular non-inl-sounding pipes failing due to this in any of Angular's list of supported browsers today?

In this case, it's not an optional thing, it's a bug. Other than https://github.com/angular/angular/issues/10764, you might have better luck just reporting that "percent pipes fails in latest safari", and mentioning that regardless of the status of the intl issue, Angular should not fail on supported browser with undocumented polyfill.

That'd all be an Angular issue though, not Angular CLI issue. The AngularCLI needs a source of truth for things like these, and I guess https://github.com/angular/quickstart or quickstart plunker demo source could be that source. If the polyfill is not in them, then it's an Angular issue. Once the polyfill is in them, then it becomes and AngularCLI issue.

filipesilva commented 8 years ago

This is the source of truth we use actually: https://angular.io/docs/ts/latest/guide/browser-support.html

intl is listed in the optional section so we don't include it by default.

At the time we didn't have this page I think, but since we do now and this polyfill is considered optional, I'm closing the issue. I'll mark it as a FAQ though.

JochemKuijpers commented 7 years ago

Additional information on Firefox for Android:

Firefox for Android does not support Intl even though the desktop version with identical version number does support it. It will throw the following error

EXCEPTION: Intl is not defined

The reason this support is lacking apparently comes down to APK size, which Google recently increased.

Source: https://bugzilla.mozilla.org/show_bug.cgi?id=1215247

We've held off on shipping this API on Android because we're worried about the APK size increase it will bring. We'll need to address that concern before we can enable this by default.

APK limit size was 50MB, but now, Google changes the limit to 100MB (http://android-developers.blogspot.ca/2015/09/support-for-100mb-apks-on-google-play.html).

Firefox for Android will receive Intl support in version 54.0.0 in june this year, until then, use the polyfill fix mentioned above.

hgoebl commented 7 years ago

BTW it seams there is a better way than importing into a bundle (maybe not for all cases):

https://github.com/andyearnshaw/Intl.js/#intljs-and-ft-polyfill-service

<script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=Intl.~locale.fr,Intl.~locale.pt"></script>

This is by far the best option to use the Intl polyfill since it will only load the polyfill code and the corresponding locale data when it is really needed (e.g.: safari will get the code and patch the runtime while chrome will get an empty script tag).

alexduros commented 7 years ago

Like @chompy18, I experienced the same probleme with unmatched parentheses, referenced in this issue :

https://github.com/andyearnshaw/Intl.js/issues/231#issuecomment-286450515

Forcing intlin version 1.2.4 fixed my problem.

hheexx commented 7 years ago

@hgoebl does not work with googlebot that way. Not sure why,

angular-automatic-lock-bot[bot] commented 5 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.