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

Support third-party fragments #17

Closed
jgierer12 opened this issue Nov 8, 2018 · 7 comments

Comments

@jgierer12
Copy link

@jgierer12 jgierer12 commented Nov 8, 2018

Sometimes, it is necessary to include third-party fragments, such as gatsby-transform-sharps GatsbyImageSharpFluid fragment required by the gatsby-image component. As far as I can tell, it's currently impossible to use this in a blade-generated query.

@sw-yx

This comment has been minimized.

Copy link
Owner

@sw-yx sw-yx commented Nov 9, 2018

thank you. this one may need some thinking but its not impossible

@sw-yx

This comment has been minimized.

Copy link
Owner

@sw-yx sw-yx commented Nov 9, 2018

got some sample code i can work with?

jgierer12 added a commit to jgierer12/babel-blade-17 that referenced this issue Nov 11, 2018
Init
diff --git a/README.md b/README.md
new file mode 100644
index 0000000..52ab4a6
--- /dev/null
+++ b/README.md
@@ -0,0 +1,17 @@
+# Example repo for [sw-yx/babel-blade#17](sw-yx/babel-blade#17)
+
+## `framework.js`
+
+This represents an imaginative third-party framework. We don't have control over this framework's code.
+
+The framework exports an `Image` component, which renders an image with some props.
+
+To ease the usage of this component with the framework's graphql API, it also exports a `query` function. This function is a wrapper for the user's query that adds a fragment for all the data the image needs.
+
+A real example for this would be [Gatsby](https://gatsbyjs.org/)'s [`transformer-sharp`](https://www.gatsbyjs.org/packages/gatsby-transformer-sharp/) plugin. Similarly to our imaginative framework, it adds a fragment to all queries. This can be used to query for the [`image`](https://www.gatsbyjs.org/packages/gatsby-image/) plugin's props.
+
+## `index.js`
+
+This represents our own code, which we want to use `babel-blade` on. It imports the framework's `query` function, so it can use the `FrameworkImage` fragment in its query.
+
+However, with `babel-blade`, there is currently no easy way to use this fragment.
diff --git a/framework.js b/framework.js
new file mode 100644
index 0000000..b56e9e5
--- /dev/null
+++ b/framework.js
@@ -0,0 +1,24 @@
+export const Image = ({ image }) => (
+  <img
+    src={image.src}
+    alt={image.alt}
+    width={image.size.width}
+    height={image.size.height}
+  />
+);
+
+const imageFragment = `
+  fragment FrameworkImage on Image {
+    src
+    alt
+    size {
+      width
+      height
+    }
+  }
+`;
+
+export const query = gql => {
+  gql += imageFragment;
+  return fetch(gql);
+};
diff --git a/index.js b/index.js
new file mode 100644
index 0000000..9bfdf98
--- /dev/null
+++ b/index.js
@@ -0,0 +1,15 @@
+import { Image, query } from "./framework";
+
+const pageQuery = `
+  query {
+    page {
+      image {
+        ...FrameworkImage
+      }
+    }
+  }
+`;
+
+export default () => {
+  query(pageQuery).then(data => <Image image={data.page.image} />);
+};
@jgierer12

This comment has been minimized.

Copy link
Author

@jgierer12 jgierer12 commented Nov 11, 2018

Sure, I created a quick example repo to better illustrate the problem: https://github.com/jgierer12/babel-blade-17

I think the implementation of this feature would be trivial, we simply have to add the name of the fragment to the query. But it is incompatible with the current API, since all string parameters after the first one are assumed to be decorators.

If you would be willing to introduce a breaking API change for this, making the second parameter an array of decorators instead would make sense. Otherwise, perhaps something like

data.field(args, ...decorators)(fragment)

might work.

@sw-yx

This comment has been minimized.

Copy link
Owner

@sw-yx sw-yx commented Nov 12, 2018

yes i am willing to introduce a breaking API change. this is still early days :)

But it is incompatible with the current API, since all string parameters after the first one are assumed to be decorators.

actually not true, i think. did you read this somewhere? it used to be the case, but now you can do

data.field('@sort', 'foo: 1', '@delay', 'bar:2')

and it'd work out to

data {
	field(foo: 1, bar: 2) @sort @delay {
		// ...
	}
}

nice, eh? proof

but it maybe too nice. i'm willing to break that flexibility for a bit more predictability and third party/inline fragments.

in terms of having curried functions like you propose it would introduce extra complexity. i propose this:

data.field(subfields, args, decorators)

all three args would take either null, strings, or template strings. subfields include fragments but also any other fields the user doesnt need to actually use in the js at that point in time.

Funny enough, this was the original API design, and i broke it to go for the flexibility which seemed like a good idea at the time. While I originally didnt like asking users to memorize the three arguments (and also putting in null for stuff they dont use), explicitness isnt the worst thing in the world, and it means that i can allow for both subfields and 3p fragments much more easily.

@jgierer12

This comment has been minimized.

Copy link
Author

@jgierer12 jgierer12 commented Nov 12, 2018

actually not true, i think. did you read this somewhere? it used to be the case, but now you can do

Ah, I didn't know it works like that. That is very cool indeed! I actually don't remember where I got that from, I think you may have said it in the video?

Anyways, I like your proposed API. Being able to manually pass in fields, not just fragments, surely would be useful too in some situations. I suggest all three args can also take arrays of their supported types, so you can for example mix inline fragments and blade-generated ones.

@sw-yx

This comment has been minimized.

Copy link
Owner

@sw-yx sw-yx commented Nov 12, 2018

ah. very interesting. sure thing.

@sw-yx

This comment has been minimized.

Copy link
Owner

@sw-yx sw-yx commented Oct 9, 2019

closing as i am no longer maintaining this - seeking maintainer! #23

@sw-yx sw-yx closed this Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.