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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update js dependencies #2156

Merged
merged 22 commits into from Nov 13, 2017

Conversation

@deivid-rodriguez
Copy link
Contributor

commented Nov 3, 2017

馃帺 What? Why?

This is me playing around with npm and react.

The tests were running into problems with react 16. However, I didn't seem to get those problems when trying out comments when using a real browser. So I tried using headless chrome for tests and it worked!

Some comments about migrating to headless chrome:

  • Headless chrome is still in early stages in it's noticeably slower than phantomjs. This might make sense since it uses a real browers instead of a virtual one, so I assume there more stuff going on behind the scenes making things slower.

  • Not all tests can be migrated since headless chrome does not yet support multiple windows. For those tests using multiple windows I've kept phantomjs around.

馃搶 Related Issues

  • Related to #2147.
  • It does basically the same thing as #2110.

馃搵 Subtasks

None.

馃摲 Screenshots (optional)

None.

馃懟 GIF

tiger

@ghost ghost assigned deivid-rodriguez Nov 3, 2017
@ghost ghost added the in-progress label Nov 3, 2017
@deivid-rodriguez deivid-rodriguez force-pushed the chore/update_js_dependencies branch from 2a5ec6e to 6eb7c85 Nov 3, 2017
@codecov

This comment has been minimized.

Copy link

commented Nov 3, 2017

Codecov Report

Merging #2156 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2156      +/-   ##
==========================================
- Coverage   98.58%   98.57%   -0.01%     
==========================================
  Files        1186     1186              
  Lines       27371    27371              
==========================================
- Hits        26983    26982       -1     
- Misses        388      389       +1
@mrcasals

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2017

@deivid-rodriguez there are errors on CircleCI because it cannot find the headless Chrome. Does this PR need #2157 to work?

@mrcasals

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2017

In #2157 you say that PR is extracted from this one, but here the circleci/config.yml file is not being modified, I guess this is the problem 馃槃

@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2017

Ooops, yes. I thought both were finished by they still need work to pass on CircleCI!

@deivid-rodriguez deivid-rodriguez force-pushed the chore/update_js_dependencies branch 2 times, most recently from 6714ca5 to 931d23d Nov 7, 2017
@deivid-rodriguez deivid-rodriguez force-pushed the chore/update_js_dependencies branch from 931d23d to 350df37 Nov 11, 2017
```
鈼 Console

console.error node_modules/react-dom/node_modules/fbjs/lib/warning.js:33
Warning: React depends on requestAnimationFrame. Make sure that you load
a polyfill in older browsers. http://fb.me/react-polyfills
```
@deivid-rodriguez deivid-rodriguez force-pushed the chore/update_js_dependencies branch from fd4a23d to 004b033 Nov 11, 2017
Copy link
Contributor

left a comment

Let's do this!

@@ -1,6 +1,6 @@
{
"presets": [
["es2015", { "modules": false }],

This comment has been minimized.

Copy link
@josepjaume

josepjaume Nov 13, 2017

Contributor

I also tried this in my update dependencies PR, but didn't have any real effect. Did you try to recompile the bundle.js file and saw any differences? Maybe I was doing it wrong!

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Nov 13, 2017

Author Contributor

Actually I was running into failures after upgrading to react 16, some JS tests where failing due to something about es2015 not found. So I cherry-pick the commit from your branch and it fixed it... 馃

@josepjaume josepjaume changed the title Update js dependencies (another take) Update js dependencies Nov 13, 2017
@josepjaume josepjaume merged commit 0b4e6a3 into master Nov 13, 2017
32 checks passed
32 checks passed
ci/circleci: accountability Your tests passed on CircleCI!
Details
ci/circleci: admin Your tests passed on CircleCI!
Details
ci/circleci: api Your tests passed on CircleCI!
Details
ci/circleci: assemblies Your tests passed on CircleCI!
Details
ci/circleci: budgets Your tests passed on CircleCI!
Details
ci/circleci: build_docker_images Your tests passed on CircleCI!
Details
ci/circleci: build_test_app Your tests passed on CircleCI!
Details
ci/circleci: comments Your tests passed on CircleCI!
Details
ci/circleci: core Your tests passed on CircleCI!
Details
ci/circleci: main Your tests passed on CircleCI!
Details
ci/circleci: meetings Your tests passed on CircleCI!
Details
ci/circleci: pages Your tests passed on CircleCI!
Details
ci/circleci: processes Your tests passed on CircleCI!
Details
ci/circleci: proposals Your tests passed on CircleCI!
Details
ci/circleci: surveys Your tests passed on CircleCI!
Details
ci/circleci: system Your tests passed on CircleCI!
Details
codeclimate All good!
Details
codecov/patch Coverage not affected when comparing dea9acb...004b033
Details
codecov/project 98.57% (-0.01%) compared to dea9acb
Details
codecov/project/accountability 100% remains the same compared to dea9acb
Details
codecov/project/admin 100% remains the same compared to dea9acb
Details
codecov/project/api 100% remains the same compared to dea9acb
Details
codecov/project/assemblies 100% remains the same compared to dea9acb
Details
codecov/project/budgets 100% remains the same compared to dea9acb
Details
codecov/project/comments 100% remains the same compared to dea9acb
Details
codecov/project/core 100% remains the same compared to dea9acb
Details
codecov/project/meetings 100% remains the same compared to dea9acb
Details
codecov/project/pages 100% remains the same compared to dea9acb
Details
codecov/project/processes 100% remains the same compared to dea9acb
Details
codecov/project/proposals 100% remains the same compared to dea9acb
Details
codecov/project/surveys 100% remains the same compared to dea9acb
Details
codecov/project/system 100% remains the same compared to dea9acb
Details
@josepjaume josepjaume deleted the chore/update_js_dependencies branch Nov 13, 2017
@ghost ghost removed the in-progress label Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.