search

jbolda / gatsby-source-airtable

MIT License
216 stars 43 forks source link

Mapping key clarifications #32

Closed shaunbent closed 5 years ago

shaunbent commented 5 years ago

I've just run into an issue where my mappings where not correctly applying. I managed to try this down to me using the cleanedKey not the actual column name as is in Airtable.

Do you think this is a bug in the code and we should be using the cleanedKey here: https://github.com/jbolda/gatsby-source-airtable/blob/master/gatsby-node.js#L194

Or is this behavour expected?

Happy to I open a PR to either fix the code or clarify that you need to use the actual column name within your gatsby-config.js?

jbolda commented 5 years ago

Hmm, so just that I am clear...

You have a column name in Airtable that needs cleaning. It comes in, gets "cleaned" and changes. In the mapping, you provided a name to match it. To match, you used the cleaned name, not the original name. Just high-level thinking here, we don't want to force changes on your Airtable so once we hit gatsby, we should only consider and use one name everywhere for simplicity, which would mean we should use the cleaned name, both in config and in queries.

Assuming I understand correctly, I think I would consider it a bug. A side consideration though is the discovery of the cleaned key could be a problem. How did you know it got cleaned? Throwing a warning in the code that this happened might be useful, but is that overly verbose to warn every time?

shaunbent commented 5 years ago

Hello,

You understood me perfectly. I tried to use the cleaned key and it just failed silently, I guess I just assumed it would use the cleaned key and then it took a bit of debugging before I realised that it was mapping on the original name.

I found that it was using the cleaned name after changing the column name in Airtable to remove the space from the name and it started working. Then I started reading the code.

I agree that it would be good to align the API around a single key but yes discovery is likely to become an issue. Console warnings might be a little too much if they're displayed every time. Do you think just stating we always use the cleaned keys in both config and queries in the documentation is enough?

jbolda commented 5 years ago

The only other option I can think of would be to warn on key change, and also allow an option to turn it off in the table config. So while your website is in development you would see it, and when you are confident with the schema you can turn it off.

jbolda commented 5 years ago

Do you think the previous be the best path forward, @shaunbent? Would you be available for a PR?