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

Remove use of Array.prototype.includes to support older runtimes #73

Merged
merged 2 commits into from
May 31, 2017

Conversation

jtschulz
Copy link
Contributor

@jtschulz jtschulz commented May 30, 2017

Also removes babel-polyfill in favor of transform-runtime to surface these runtime-specific issues in testing.

@stubailo 👀 please

TODO:

  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests pass
  • Update CHANGELOG.md with your change
  • If this was a change that affects the external API, update the README

;)

@jtschulz jtschulz force-pushed the fix-literal-env-for-node-5 branch from 523c087 to b6fbe6c Compare May 30, 2017 23:13
@stubailo
Copy link
Contributor

(BTW - you can use [x] to put checked boxes :])

CHANGELOG.md Outdated
@@ -2,6 +2,9 @@

### vNEXT

### v0.8.3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future PRs, please don't set the version number here and in package.json - sometimes we publish several things in one version

test/index.js Outdated
@@ -1,7 +1,9 @@
// This file cannot be written with ECMAScript 2015 because it has to load
// the Babel require hook to enable ECMAScript 2015 features!
require('babel-core/register');
require('babel-polyfill');
require("babel-core").transform("code", {
plugins: ["transform-runtime"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh never mind just read the description :]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's something I can do to clarify the intention of this in the code, I'm super happy to add it.

Also removes `babel-polyfill` in favor of `transform-runtime` to surface these runtime-specific issues in testing.
@jtschulz jtschulz force-pushed the fix-literal-env-for-node-5 branch from b6fbe6c to 97de688 Compare May 31, 2017 00:44
@jtschulz
Copy link
Contributor Author

@stubailo Thanks for the feedback 💇‍♂️

@stubailo
Copy link
Contributor

LGTM, @jnwng I think it's up to you to merge!

@jnwng
Copy link
Contributor

jnwng commented May 31, 2017

thank you @PepperTeasdale!

@jnwng jnwng merged commit 3ea46fa into apollographql:master May 31, 2017
jtschulz pushed a commit to jtschulz/eslint-plugin-graphql that referenced this pull request Jul 12, 2017
…llographql#73)

Also removes `babel-polyfill` in favor of `transform-runtime` to surface these runtime-specific issues in testing.
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.

3 participants