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

fix: remove duplicate fragments (fix #90) #94

Merged
merged 3 commits into from
May 20, 2019

Conversation

FezVrasta
Copy link
Contributor

@FezVrasta FezVrasta commented May 17, 2019

With this PR, we deduplicate fragments at runtime.

I wanted to do this at build time inside Babel but I can't find a way to do it ☹️

I copied the approach from babel-plugin-graphql-tag

@@ -1,6 +1,15 @@
// @flow
import gqlTag from 'graphql-tag';
import serialize from 'babel-literal-to-ast';
import template from '@babel/template';

const uniqueFn = template.ast(`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically we append this function inside a .reduce call after the .concat we use to append the fragments.

@codecov
Copy link

codecov bot commented May 20, 2019

Codecov Report

Merging #94 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   95.83%   95.91%   +0.08%     
==========================================
  Files           5        5              
  Lines          48       49       +1     
  Branches        8        8              
==========================================
+ Hits           46       47       +1     
  Misses          2        2
Impacted Files Coverage Δ
src/utils/compileWithFragment.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87c5b9b...acca2a7. Read the comment docs.

@evenchange4
Copy link
Owner

evenchange4 commented May 20, 2019

Accept the PR for now. I have not come up with a better solution to reduce runtime, either.

@evenchange4 evenchange4 changed the title fix: dedupe fragments (fix #90) fix: remove deduplicate fragments (fix #90) May 20, 2019
@evenchange4 evenchange4 merged commit e406153 into evenchange4:master May 20, 2019
@FezVrasta FezVrasta deleted the fix/duplicate-fragments branch May 20, 2019 09:48
@FezVrasta FezVrasta changed the title fix: remove deduplicate fragments (fix #90) fix: remove duplicate fragments (fix #90) May 20, 2019
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.

2 participants