Update core-js in group stark-apps to the latest version 🚀 #1206

greenkeeper · 2019-03-19T20:02:20Z

The dependency core-js was updated from `2.6.5` to `3.0.0`.

This version is not covered by your current version range.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

Find out more about this release.

FAQ and help

There is a collection of frequently asked questions. If those don’t help, you can always ask the humans behind Greenkeeper.

Your Greenkeeper bot 🌴

coveralls · 2019-03-21T10:48:40Z

Coverage remained the same at 94.701% when pulling 649b969 on greenkeeper/stark-apps/core-js-3.0.0 into e931b15 on master.

coveralls · 2019-03-21T10:48:40Z

Coverage increased (+0.003%) to 92.801% when pulling 8da6515 on greenkeeper/stark-apps/core-js-3.0.0 into ca2c78d on master.

christophercr · 2019-03-22T13:18:20Z

@SuperITMan @cnomes can you review this PR? Apart from the commits from Greenkeeper, I've added 1 commit with these changes:

Adapt the polyfills used in showcase and starter (in the app and in the unit tests) since the structure has changed in core-js 3.0.0
Adapt the polyfills used in the unit tests of stark-core and stark-ui for the same reason: new structure in core-js 3.0.0
Add a workaround in the tsconfig.json to be able to use core-js 3.0.0 in showcase and starter because Angular CLI uses version 2 internally. See Support [email protected] angular/angular-cli#13954 and 3.0.0-beta.3 - Can't resolve 'core-js/es7/reflect' zloirock/core-js#412 (comment)
Add core-js as a dependency at the root of the project to be able to use it in the unit tests of stark-core and stark-ui (so far it has been working "by coincidence" because they were using core-js v 2 from a transient dependency)

SuperITMan · 2019-03-22T15:24:10Z

I guess it's a breaking change, maybe we should mention it in our CHANGELOG. What do you think ?

christophercr · 2019-03-22T15:26:35Z

Mmm not really, because core-js is used only for the showcase and the starter... in our stark-packages we use it only for our unit tests so we can run them also in IE 11.

The apps based in Stark can continue using core-js v2 as usual... again... stark itself does not depend on core-js

carlo-nomes

A couple of notes / questions. Also I'm wondering if importing core-js/es does not affect our bundle size to much?

starter/tsconfig.json

carlo-nomes · 2019-03-27T16:37:11Z

showcase/tsconfig.json

-      "@nationalbankbelgium/*": ["../node_modules/@nationalbankbelgium/*"]
+      "@nationalbankbelgium/*": ["../node_modules/@nationalbankbelgium/*"],
+      "core-js/es7/reflect": ["../node_modules/core-js/proposals/reflect-metadata"],
+      "core-js/es6/*": ["../node_modules/core-js/es/*"]


are these needed? because it seems you removed them from starter/src/polyfills.browser.ts

In fact this is the workaround to get Angular CLI working with the latest core-js... See angular/angular-cli#13954... I would like to add a FIXME somewhere to mention this but where do I add it? This is a json file and we cannot put comments in here :(

This is also related to the issue you mentioned in the other remarks: angular/angular#29531

Ok, I understand the fix now. 😄

As for the .json comments. You are right we shouldn't/can't comment there. Maybe we should start a known issues section in our README.md I actually think this would also provide better visibility to active issues (for us) then the FIXME comments we add in code. 🤔
@SuperITMan @christophercr what do you think?

Thinking about it again known issues might be the wrong title. 🤔

I think it could be a good idea 😊
I'll have a look asap

I added a comment about this workaround in the POLYFILLS.md and also in polyfills.browser.ts of the Showcase and Starter.

So I guess the idea from @cnomes will be done in a different PR right?

Probably best. 😄

carlo-nomes · 2019-03-27T16:38:11Z

packages/stark-ui/base.spec.ts

+import "../../node_modules/zone.js/dist/sync-test";
+import "../../node_modules/zone.js/dist/jasmine-patch"; // put here since zone.js 0.6.14
+import "../../node_modules/zone.js/dist/async-test";
+import "../../node_modules/zone.js/dist/fake-async-test";


Could you add a FIXME comment here? you can reference angular/angular#29531

Well, in fact, this is not related to Angular, it is just the fact that we now have core-js as a direct dependency in our root package. Before we didn't and it was just working fine by coincidence because core-js was a transient dependency.

So I think it is better like this now regardless of which version of core-js is used. What do you think?

Mmm I just saw you were talking about zone.js. Well, it is the same reason, we explicitly point to the package at the root which we already have as a direct dependency 😉 I think it is safer this way and easier to troubleshoot

I think it would be better that we add zone.js and core.js to our tsconfig.spec.jsonsince we already do this for some other dependencies.

Then something not really related to this PR....
If we add something like

"paths": { "*": ["../stark-core/node_modules/*", "../../node_modules/*", ...], }

We should hit all these indirect dependencies in one go. But we should investigate if it has a significant impact on our build process.

Also I think we should review our dependency management / use of transient dependencies. But I will move that discussion to another issue. 😄

I added #1224

packages/stark-core/base.spec.ts

christophercr · 2019-03-28T11:04:30Z

About the bundle size, indeed, I will refine the imports of ``core-js` to the strictly neccessary ones ;)

christophercr · 2019-04-01T15:50:56Z

@SuperITMan @cnomes I've updated this PR:

added a FIXME comment to the polyfills.browser.ts file in Showcase and Starter about the workaround in tsconfig.json
added the path mappings to paths in tsconfig-build.json of stark-core and stark-ui to align with what we already do with some other root dependencies
the import from core-js/es could be refined but the diff in kbytes is minimal so I prefer to leave it like that so we are covered in case we might need some other standard ES features in the future

carlo-nomes

LGTM 👍

Closes #1206

greenkeeper · 2019-04-06T07:26:04Z

The dependency core-js was updated from 2.6.5 to 3.0.1.

Update to these versions instead 🚀

Release Notes for 3.0.1 - 2019.04.06

Fixed some cases of work with malformed URI sequences in URLSearchParams, #525
Added a workaround for a rollup issue, #513

christophercr · 2019-04-08T13:01:28Z

@SuperITMan I've updated this PR with the docs about polyfills in the POLYFILLS.md and also in the "Getting Started" in the showcase. Could you have a look?

SuperITMan · 2019-04-30T07:38:51Z

packages/stark-core/tsconfig-build.json

@@ -11,7 +11,9 @@
      "@angular/router": ["../../node_modules/@angular/router"],
      "rxjs/*": ["../../node_modules/rxjs/*"],
      "environments/environment": ["./src/common/environment"],
-      "@nationalbankbelgium/stark-core": ["."]
+      "@nationalbankbelgium/stark-core": ["."],


Previously, @nationalbankbelgium/stark-<package>": ["."], was always the last mentioned path, is not it important so ? 😊

Technically speaking, no... it is not important. What could be important is a path like this: "*" : ["some-path"] which indeed should be at the end because the TS compiler checks these paths sequentially from top to bottom, so the more generic paths should be at the end to prevent overlapping with the rest of paths.

But that is not the case here 😉

Okido. Thanks for the explanation 😊
In this case, could we change the order like this ?

"paths": { "@angular/common": ["../../node_modules/@angular/common"], "@angular/core": ["../../node_modules/@angular/core"], "@angular/router": ["../../node_modules/@angular/router"], "@nationalbankbelgium/stark-core": ["."], "core-js/*" : ["../../node_modules/core-js/*"], "environments/environment": ["./src/common/environment"], "rxjs/*": ["../../node_modules/rxjs/*"], "zone.js/*": ["../../node_modules/zone.js/*"] },

Otherwise I find this a bit weird if it's not sorted in alphabetical order. What do you think ? 😊

True, I forgot to sort it... I'll do it ;)

SuperITMan · 2019-04-30T07:39:17Z

packages/stark-rbac/tsconfig-build.json

@@ -23,7 +23,9 @@
      "rxjs/*": ["../../node_modules/rxjs/*"],
      "environments/environment": ["../../dist/packages/stark-core/src/common/environment"],
      "@nationalbankbelgium/stark-core": ["../../dist/packages/stark-core"],
-      "@nationalbankbelgium/stark-rbac": ["."]
+      "@nationalbankbelgium/stark-rbac": ["."],


same here about order

See my previous remark

SuperITMan · 2019-04-30T07:39:27Z

packages/stark-ui/tsconfig-build.json

@@ -29,7 +29,9 @@
      "rxjs/*": ["../../node_modules/rxjs/*"],
      "environments/environment": ["../../dist/packages/stark-core/src/common/environment"],
      "@nationalbankbelgium/stark-core": ["../../dist/packages/stark-core"],
-      "@nationalbankbelgium/stark-ui": ["."]
+      "@nationalbankbelgium/stark-ui": ["."],


same here about order

See my previous remark

SuperITMan · 2019-04-30T07:41:17Z

showcase/tsconfig.json

-      "@nationalbankbelgium/*": ["../node_modules/@nationalbankbelgium/*"]
+      "@nationalbankbelgium/*": ["../node_modules/@nationalbankbelgium/*"],
+      "core-js/es7/reflect": ["../node_modules/core-js/proposals/reflect-metadata"],
+      "core-js/es6/*": ["../node_modules/core-js/es/*"]


I think it could be a good idea 😊
I'll have a look asap

…w structure in core-js 3.0.0

christophercr · 2019-04-30T13:29:53Z

@SuperITMan PR updated

greenkeeper bot added the greenkeeper label Mar 19, 2019

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch 2 times, most recently from 6b7de89 to 649b969 Compare March 21, 2019 10:08

christophercr requested review from SuperITMan and carlo-nomes March 22, 2019 13:18

christophercr added this to the 10.0.0-beta.7 milestone Mar 22, 2019

christophercr added comp: stark-starter Stark Starter comp: stark-showcase type: feature labels Mar 22, 2019

carlo-nomes reviewed Mar 27, 2019

View reviewed changes

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch 2 times, most recently from 4c23a38 to a3987b6 Compare April 1, 2019 15:44

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch from a3987b6 to 3b39051 Compare April 2, 2019 10:40

christophercr modified the milestones: 10.0.0-beta.7, 10.0.0-beta.8 Apr 2, 2019

carlo-nomes approved these changes Apr 3, 2019

View reviewed changes

greenkeeper bot added a commit that referenced this pull request Apr 6, 2019

chore(package): update core-js to version 3.0.1

8c00532

Closes #1206

greenkeeper bot added a commit that referenced this pull request Apr 6, 2019

chore(package): update core-js to version 3.0.1

84fe986

Closes #1206

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch 3 times, most recently from 9871876 to c7ff82d Compare April 8, 2019 12:57

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch from c7ff82d to d4173ac Compare April 8, 2019 15:32

dsebastien mentioned this pull request Apr 16, 2019

Repo setup inrupt/wac-ldp#6

Merged

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch from d4173ac to 4edc63a Compare April 24, 2019 12:51

greenkeeper bot added 3 commits April 24, 2019 16:03

chore(package): update core-js to version 3.0.0

a4e24c2

chore(package): update core-js to version 3.0.0

3f9d4e9

chore(package): update lockfile showcase/package-lock.json

48b1d99

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch 2 times, most recently from 3734809 to 702ccee Compare April 24, 2019 15:32

SuperITMan reviewed Apr 30, 2019

View reviewed changes

feat(stark-demo): adapt polyfills (app and tests) according to the ne…

8da6515

…w structure in core-js 3.0.0

christophercr force-pushed the greenkeeper/stark-apps/core-js-3.0.0 branch from 702ccee to 8da6515 Compare April 30, 2019 12:51

SuperITMan approved these changes Apr 30, 2019

View reviewed changes

SuperITMan merged commit 152f5d3 into master Apr 30, 2019

SuperITMan deleted the greenkeeper/stark-apps/core-js-3.0.0 branch April 30, 2019 17:44

SuperITMan mentioned this pull request Jun 28, 2023

[Snyk] Fix for 1 vulnerabilities #3616

Closed

SuperITMan mentioned this pull request Jul 8, 2023

[Snyk] Security upgrade @angular-builders/custom-webpack from 12.1.3 to 14.0.0 #3623

Closed

SuperITMan mentioned this pull request Feb 11, 2024

[Snyk] Fix for 1 vulnerabilities #3735

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update core-js in group stark-apps to the latest version 🚀 #1206

Update core-js in group stark-apps to the latest version 🚀 #1206

greenkeeper bot commented Mar 19, 2019

coveralls commented Mar 21, 2019

coveralls commented Mar 21, 2019 •

edited

Loading

christophercr commented Mar 22, 2019 •

edited

Loading

SuperITMan commented Mar 22, 2019

christophercr commented Mar 22, 2019 •

edited

Loading

carlo-nomes left a comment

carlo-nomes Mar 27, 2019

christophercr Mar 27, 2019

christophercr Mar 27, 2019

carlo-nomes Mar 28, 2019 •

edited

Loading

carlo-nomes Mar 28, 2019

SuperITMan Apr 30, 2019

christophercr Apr 30, 2019 •

edited

Loading

carlo-nomes Apr 30, 2019

carlo-nomes Mar 27, 2019

christophercr Mar 27, 2019 •

edited

Loading

christophercr Mar 27, 2019

carlo-nomes Mar 28, 2019 •

edited

Loading

carlo-nomes Mar 28, 2019

christophercr commented Mar 28, 2019

christophercr commented Apr 1, 2019

carlo-nomes left a comment

greenkeeper bot commented Apr 6, 2019

christophercr commented Apr 8, 2019 •

edited

Loading

SuperITMan Apr 30, 2019

christophercr Apr 30, 2019 •

edited

Loading

SuperITMan Apr 30, 2019

christophercr Apr 30, 2019

SuperITMan Apr 30, 2019

christophercr Apr 30, 2019

SuperITMan Apr 30, 2019

christophercr Apr 30, 2019

SuperITMan Apr 30, 2019

christophercr commented Apr 30, 2019

Update core-js in group stark-apps to the latest version 🚀 #1206

Update core-js in group stark-apps to the latest version 🚀 #1206

Conversation

greenkeeper bot commented Mar 19, 2019

The dependency core-js was updated from 2.6.5 to 3.0.0.

coveralls commented Mar 21, 2019

coveralls commented Mar 21, 2019 • edited Loading

christophercr commented Mar 22, 2019 • edited Loading

SuperITMan commented Mar 22, 2019

christophercr commented Mar 22, 2019 • edited Loading

carlo-nomes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlo-nomes Mar 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophercr Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophercr Mar 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlo-nomes Mar 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophercr commented Mar 28, 2019

christophercr commented Apr 1, 2019

carlo-nomes left a comment

Choose a reason for hiding this comment

greenkeeper bot commented Apr 6, 2019

christophercr commented Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

christophercr Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophercr commented Apr 30, 2019

The dependency core-js was updated from `2.6.5` to `3.0.0`.

coveralls commented Mar 21, 2019 •

edited

Loading

christophercr commented Mar 22, 2019 •

edited

Loading

christophercr commented Mar 22, 2019 •

edited

Loading

carlo-nomes Mar 28, 2019 •

edited

Loading

christophercr Apr 30, 2019 •

edited

Loading

christophercr Mar 27, 2019 •

edited

Loading

carlo-nomes Mar 28, 2019 •

edited

Loading

christophercr commented Apr 8, 2019 •

edited

Loading

christophercr Apr 30, 2019 •

edited

Loading