-
-
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
Option to make tables separate node types #52
Option to make tables separate node types #52
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A boolean, interesting. I think I do like it. It makes queryName
actually useful still. We will want to drill this down into the buildNode
function on L306+. Those AirtableField
types hold the nodes built from mapping
. If we don't drill it down there, we will still potentially get conflicting types when using mapping
.
Looks like this should keep things backwards compat 👍 .
Good catch, added. |
Hmm, well Travis is failing and it does appear to be related, but I haven't figured what the issue is. Could you add the small example checking both |
README.md
Outdated
@@ -34,6 +34,7 @@ plugins: [ | |||
tableName: `YOUR_TABLE_NAME`, | |||
tableView: `YOUR_TABLE_VIEW_NAME`, // optional | |||
queryName: `OPTIONAL_NAME_TO_IDENTIFY_TABLE`, // optional | |||
createSeparateNodeType: true, // optional - makes all records in this table a separate node type, based on your tableView, or if not present, tableName, e.g. a table called "Fruit" would become "allAirtableFruit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth mentioning that the default is false. Also, we should probably add another section in the docs on why you would care to do this. We could even point back to the issues regarding it.
gatsby-node.js
Outdated
@@ -146,6 +148,10 @@ exports.sourceNodes = async ( | |||
store, | |||
cache | |||
}); | |||
|
|||
const nodeType = (row.separateNodeType) ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only issue I can think of is that that isn't falsey when we are expecting it to be (in regards to the failing test). Could it be an issue with an unresolved promise?
Oh, and let's do a minor bump on this. Feel free to include that in this PR. |
Will do! We just did a big launch so I'll probably not get to it until next week. |
Just checking in to make sure you are not waiting on a review. |
Checking in on this again. Still busy with that launch? |
Any chance this is still possible to add in? Would love to be able to separate tables by node name. I'd be willing to take this on if no one else is intereseted. Thanks |
I would love to see this get in @ndrabins. We can merge this PR into a sperate feature branch and you can work off that. Otherwise you can start fresh if that seems to best course of action. |
Awesome, sounds good @jbolda. I should be able to get to this either this weekend or next. |
@jbolda hey! So I tried working on this PR this weekend starting from here. Immediately hit a roadblock in that even when I Also I haven't tested it yet, but this PR seems pretty solid to my beginner eyes. Was there something specific the original author was missing or just a broken test? |
@ndrabins I think it is pretty close. We needed to get the existing tests to pass and add a new test to check this implementation. Hmm, that is pretty odd. We are working through an issue on the newest gatsby via #117 that may be related? Since it sounds like you are in agreement that what is here is good so far, I will go ahead and merge this into a separate feature branch sometime today that we can work on. Sound good to you? |
Yea that sounds good. I'll check that out then and go from there. Will try again to see if I can get the linking to work. Thanks! |
* Option to make tables separate node types (#52) * Option to make tables separate node types * Revert "Option to make tables separate node types" This reverts commit b59ffda. * Removed package/lock from branch * Added AirtableField node types. * Added more info for createSeparateNodeType * Version bump * Check that separateNodeType option exists * add option to set mapping fields each as a separate type (#115) * separateMapTypes config option * update version * Feat/multi node types/parent node field (#144) * prettier all the things * add separateNodeType option * spelling mistake * add separateNodeType * add example that errors without new features * bump version number Co-authored-by: Kevin Miller <keveemiller@gmail.com>
Per conversation in #51, this PR:
createSeparateNodeType
boolean option to tablesAirtable
node type (so a tableFruit
would be underallAirtableFruit
)