-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Added field prefix table option #51
Conversation
@kevee can you re-push your changes without whatever code formatting tool you have automatically formatting everything? Or change whatever you’re using (prettier?) to adhere to the existing formatting? Previewing the changes there are a ton of extraneous changed lines and the PR should be clean with only the changes you’re making. Thanks! |
@jamessimone Sorry didn't review the diff before committing. Fixed. |
@kevee thank you kindly. I am traveling at the moment but hopefully one of us will be able to review this shortly. It’s a great workaround to this limitation in Airtable and always great to see contributions! |
Yes, this should resolve both those issues. |
So my initial thought was to append a value to the node type. Is there more value in that or in your implementation (or both)? For clarity, prepending (see
vs appending (see
Let me know if you need further clarification on anything. |
I do like adding a suffix the node type instead of the field, since I am working with so many Airtable queries that look like: allAirtable(
filter: { queryName: { eq: "MyTableQueryName" } }
...
) But I think that it would make the Airtable plugin operate differently than, say, gatsby-source-filesystem, where since all the nodes are the same thing they get pulled under allFile(filter: { sourceInstanceName: { eq: "data" } }) { However, unlike gatsby-source-filesystem, each airtable is basically creating a different schema, which I think would mean that they are different node types. If we add suffixes to the node type, it would also solve the minor, yet annoying, issue where in the GraphiQL interface it pulls all of the airtable fields (which in my case is over 100) for autocomplete. If the suffix was optional, there wouldn't be issues with migration to this new feature. This PR was the result of my not wanting to pick sides on that issue, so instead it introduces what I'm thinking of as like SQL column name resolution notation ( |
Yea there is definitely some precedent to keep them all the same type, or at least there was early on. We would need to make this optional whichever way we choose so I'm not worried if this plugin is the only one to do it this way. Especially so as we essentially have a completely configurable schema. So it sounds like we definitely want to do configurable type. Do you think it's still worthwhile to configure the field as well? It seems like you have more basis for an opinion on that than I. (@jamessimone or others feel free to express any opinions you may have as well.) In other considerations, I think we do a minor bump for this. Also, if you wouldn't mind committing an small example for this so we can use it for "testing" in Travis that we don't accidentally break this going forward. We will also need docs, but I'm fine worry about that last. |
@jbolda wish I could comment in greater depth but have been on an extended trip with a lot of traveling. I wanted to weigh in on the original version of what was presented just because the diff was much larger than what it should have been and that much I could see from even my phone, but I would want to take a more in-depth look at things prior to formally weighing in, which isn’t possible this week. I trust your judgement! |
I think the field prefixes would not be needed if we could create different node types per table, since they were merely a way to work around the global namespacing of fields. Since a separate node type would be a different feature entirely, I'll create a new branch and PR for that. |
Oh no problem about that @jamessimone, didn't realize you were still traveling. Thanks @kevee for jumping on that. Getting some 👀 on it. |
Closing this in favor of #52 |
When you have two tables in the same or different bases with the same field name, but different field types, then Graphql will not create the fields at all. We have a project where we pull from multiple bases we have little control over, and wanted to prefix the field names from different tables. This also helps prevent confusion when dealing with several different tables across projects.
This PR adds a new
fieldPrefix
option to tables that will get appended to any fields in a table. It works fine with linked table fields as well.