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

Removed use of `createFragment` and bumped AC version #357

Merged
merged 6 commits into from Dec 9, 2016

Conversation

Projects
None yet
4 participants
@tmeasday
Copy link
Contributor

tmeasday commented Dec 6, 2016

For #356.

@jbaxleyiii any idea why this test is failing? It's the "real" SSR test; it seems to be throwing an error out of whatwg-fetch, and started when I updated to latest AC. A couple of things:

  1. Latest AC bumped to whatwg-fetch@2.0.0.

  2. This test shouldn't use whatwg-fetch at all, given it's node code, it should use isomorphic-fetch. I wonder if Jest is somehow doing it with JSDOM stuff (that's in the stack trace too). I'm out of my depth with this stuff.

Removed use of `createFragment` and bumped AC version
We don't actually use the fragment output of `parser` but if we did, it'd be more sensible to just deal in vanilla `graphql` documents I expect.

For #356
@tmeasday

This comment has been minimized.

Copy link
Contributor Author

tmeasday commented Dec 6, 2016

Also, you might want to consider the PR too ;)

@tmeasday

This comment has been minimized.

Copy link
Contributor Author

tmeasday commented Dec 6, 2016

Oh, weird, the tests passed, just the tsc failed. The "SSR › renderToStringWithData › should work on a non trivial example" test fails here though

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Dec 6, 2016

@tmeasday sadly I'll have to get to this tomorrow :( I've got an update to out giving platform I need to babysit out tonight. I'll work on cleaning up some of the test output as well to make it clearer when I work on it tomorrow!

Also, thank you SO much for helping to support this project. I've found my able time reduced recently due to some changes at work and you helping maintain it has been incredible. So thanks 😻

tmeasday added some commits Dec 6, 2016

@tmeasday

This comment has been minimized.

Copy link
Contributor Author

tmeasday commented Dec 6, 2016

I fixed most of the problems but the test still fails locally and there are some weird other problems. Would appreciate you taking a look.

@SimenB

This comment has been minimized.

Copy link

SimenB commented Dec 8, 2016

Any news here? It messes up my pristine output 😄 Getting errors I can do nothing about sucks, and chrome keeps telling me I get warnings in the console...

James Baxley
@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Dec 9, 2016

@tmeasday all fixed! I'm going to merge this through

@jbaxleyiii jbaxleyiii merged commit fc3ec89 into master Dec 9, 2016

5 checks passed

./dist/index.min.js -162 bytes (-0.58%) → 27,936 bytes
CLA Author has signed the Meteor CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.4%) to 94.378%
Details

@jbaxleyiii jbaxleyiii deleted the 356-fix-fragment-warnings branch Dec 9, 2016

@zol zol removed the in progress label Dec 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment