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

Removes runtime dependency babel-polyfill #2692

Merged
merged 2 commits into from
Sep 17, 2016
Merged

Conversation

flovilmart
Copy link
Contributor

@flovilmart flovilmart commented Sep 10, 2016

Was originally added here: https://github.com/ParsePlatform/parse-server/pull/543/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R14

But we don't need those because:

  1. during test, we use babel-node as the runner, so it's already injected
  2. during prod, it's transpiled with babel, so should be compatible with node 4.3.

Adresses stack traces from #2654 that would use a polyfill version of the stringify that's slower

See:

screenshot 2016-09-07 11 41 05

@codecov-io
Copy link

codecov-io commented Sep 10, 2016

Current coverage is 92.22% (diff: 100%)

Merging #2692 into master will decrease coverage by <.01%

@@             master      #2692   diff @@
==========================================
  Files           102        102          
  Lines         12446      12443     -3   
  Methods        1559       1559          
  Messages          0          0          
  Branches       2040       2038     -2   
==========================================
- Hits          11480      11476     -4   
- Misses          966        967     +1   
  Partials          0          0          

Powered by Codecov. Last update cab69b4...c17c5af

@flovilmart flovilmart modified the milestone: 2.2.20 Sep 10, 2016
@zingano
Copy link

zingano commented Sep 15, 2016

When I run this branch I get the following errors. It looks to me as though other modules still need babel-polyfill and don't have it. Probably I did something wrong in moving to this branch - I'm most recently an iOS engineer, before than an OS one (Symbian kernel) - therefore fairly new to node.js and everything server-side.

What I did was fork parse-server, checkout your branch, confirmed that babel-polyfill was missed, then npm install, and then copy that whole parse-server directory replacing the parse-server directory in my source tree that is actually pushed to Heroku. (Usually I just use the package.json to allow Heroku to build it, but npm install it when I need to do things like this.)

******* Starting Parse Server in environment: development *********
2016-09-15T09:44:02.421635+00:00 app[web.1]: parse-server: databaseURI <redacted>&connectTimeoutMS=6000&ssl=false&socketTimeoutMS=0&maxPoolSize=500&auto_reconnect=true
2016-09-15T09:44:02.421642+00:00 app[web.1]: parse-server: serverURL <redacted>
2016-09-15T09:44:02.421702+00:00 app[web.1]: parse-server: masterKey <redacted>
2016-09-15T09:44:02.421795+00:00 app[web.1]: parse-server: fileKey <redacted>
2016-09-15T09:44:04.790124+00:00 app[web.1]: parse-server: mounted on /parse
2016-09-15T09:44:04.797409+00:00 app[web.1]: Parse Server running on port 33510.
2016-09-15T09:44:05.268838+00:00 heroku[web.1]: State changed from starting to up
2016-09-15T09:44:29.559074+00:00 app[web.1]: �[36mverbose�[39m: REQUEST for [GET] /parse/login: {
2016-09-15T09:44:29.559106+00:00 app[web.1]:   "password": <redacted>,
2016-09-15T09:44:29.559107+00:00 app[web.1]:   "username": <redacted>
2016-09-15T09:44:29.559109+00:00 app[web.1]: } method=GET, url=/parse/login, host=<redacted>.herokuapp.com, connection=close, x-parse-client-version=osx1.12.0, x-parse-revocable-session=1, accept=*/*, x-parse-application-id=<redacted>, x-parse-client-key=<redacted>, x-parse-installation-id=<redacted>, x-parse-os-version=10.11.6 (15G1004), accept-language=en-gb, accept-encoding=gzip, deflate, content-type=application/json; charset=utf-8, user-agent=Task360%20Admin/140 CFNetwork/760.6.3 Darwin/15.6.0 (x86_64), x-parse-app-build-version=140, x-parse-app-display-version=3.0, x-request-id=115a02ba-64bb-4efc-8891-b577c5d50de5, x-forwarded-for=178.238.149.43, x-forwarded-proto=https, x-forwarded-port=443, via=1.1 vegur, connect-time=1, x-request-start=1473932669380, total-route-time=0, content-length=60, password=sushi4me, username=zingano
2016-09-15T09:44:29.606773+00:00 app[web.1]: �[31merror�[39m: Error generating response. [TypeError: specialQuerykeys.includes is not a function] 
2016-09-15T09:44:29.609823+00:00 app[web.1]: �[31merror�[39m: Uncaught internal server error. [TypeError: specialQuerykeys.includes is not a function] TypeError: specialQuerykeys.includes is not a function
2016-09-15T09:44:29.609827+00:00 app[web.1]:     at validateQuery (/app/node_modules/parse-server/lib/Controllers/DatabaseController.js:104:22)
2016-09-15T09:44:29.609829+00:00 app[web.1]:     at process._tickDomainCallback (node.js:407:9)
2016-09-15T09:44:29.624361+00:00 app[web.1]: TypeError: specialQuerykeys.includes is not a function
2016-09-15T09:44:29.624364+00:00 app[web.1]:     at /app/node_modules/parse-server/lib/Controllers/DatabaseController.js:112:27
2016-09-15T09:44:29.624368+00:00 app[web.1]:     at Array.forEach (native)
2016-09-15T09:44:29.624369+00:00 app[web.1]:     at validateQuery (/app/node_modules/parse-server/lib/Controllers/DatabaseController.js:104:22)
2016-09-15T09:44:29.624370+00:00 app[web.1]:     at /app/node_modules/parse-server/lib/Controllers/DatabaseController.js:946:9
2016-09-15T09:44:29.624370+00:00 app[web.1]:     at process._tickDomainCallback (node.js:407:9)

@ghost
Copy link

ghost commented Sep 15, 2016

@flovilmart updated the pull request - view changes

@flovilmart
Copy link
Contributor Author

flovilmart commented Sep 15, 2016

@zingano I just removed all references to the array includes that should have been polyfilled at runtime. If you update the branch and redeploy you should be all fine.

@drew-gross drew-gross merged commit 90e9994 into master Sep 17, 2016
@flovilmart flovilmart deleted the remove-babel-polyfill branch September 17, 2016 22:41
flovilmart added a commit that referenced this pull request Sep 18, 2016
* Removes runtime dependency babel-polyfill

* removes references to polyfilled array includes
@zingano
Copy link

zingano commented Sep 20, 2016

I just got time to test this on my dev branch and I'm seeing masses of logging, even though I have VERBOSE = false. Don't have time to look into this now but I'm not going to move this to production so won't be able to see results on a large DB. On test over 50% of the time, not surprisingly, is spent in lib/async.js, called from logger/winston.js

screenshot 2016-09-20 09 07 25

@flovilmart
Copy link
Contributor Author

We have reverted that PR as it was causing more issues. A future Pr will remove those runtime polyfills.

@flovilmart
Copy link
Contributor Author

Also, to disable VERBOSE logging, you should unset the environnement variable, setting to any value will enable VERBOSE logging

flovilmart added a commit that referenced this pull request Sep 24, 2016
* Removes runtime dependency babel-polyfill (#2692)

* Removes runtime dependency babel-polyfill

* removes references to polyfilled array includes

* Better support for polyfilling

* Removes unnecessary log

* Adds killswitch if tests are polyfilled

* Reverts usage of includes on strings
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