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

Images with a filename ending in "..." are not given the right extension when mapping to a fileNode #170

Closed
dgrcode opened this issue Mar 21, 2020 · 6 comments
Labels
stale Issue is stale. Closes automatically without activity. Apply "ongoing issue" label to prevent close.

Comments

@dgrcode
Copy link
Contributor

dgrcode commented Mar 21, 2020

Working with @carletex on a project we were hitting a problem trying to use images from airtable using gastby-image. We dug a bit and found the root cause. I'll try to describe here what we found out so we can discuss if the problem comes from this repo or gatsby-source-filesystem.

Problem

There are 3 possible scenarios –as far as we know– for filenames:

  • Filename with the right extension, e.g. attachment.jpg
  • Filename without extension, e.g. The name of the attachment
  • Filename ellipsized because it's too long, e.g. A very long name for my attach...

Attachments with ellipsized names will result in the fileNode not having the right extension. The other two cases will successfully be created with the right extension.

Root cause

We rely on gatsby-source-filesystem to figure out the file extension, even when we have the mime type of the attachment in the payload we get from airtable. In gatsby-node, line 303, we're calling createRemoteFileNode from gatsby-source-filesystem, but we don't send the ext argument.

Possible solutions

We thought about two possible solutions for this issue:

  1. If this is a more generalised problem, not only from airtable, we should probably open a PR and fix the way gatsby-source-filesystem figures out the extension of the file. Edit: I opened gatsby-source-filesystem returns '.' as the extension when the url ends in '...' gatsbyjs/gatsby#22463 to track this option
  2. If ellipsized filenames are not common outside airtable, we should probably open a PR here and send the ext property when we call createRemoteFileNode. We could use a library like Node mime, that would add 2.5kb to the package size if we use the lite version.

Temporary hack

We implemented something like the following, but this is just a hack that solves for our use case. That would replace the call to createRemoteFileNode in gatsby-node line 303

let ext

if (attachment.type.split('/')[0] === 'image') {
  ext = '.' + attachment.type.split('/').slice(-1)
}

let attachmentNode = createRemoteFileNode({
  url: attachment.url,
  store,
  cache,
  createNode,
  createNodeId,
  ext,
})

What are maintainer thoughts about next steps?

@jbolda
Copy link
Owner

jbolda commented Mar 21, 2020

This is some great work 👏 . Thanks for the effort ❤️ .

I guess I have two thoughts about this. It feels like it would be good to deal with in gatsby-source-filesystem for the reasons you have pointed out.

That being said, if Airtable is correctly sending the ext, it may be worth passing that as well. I can't think of any downside to passing it through. It would probably be considered a bugfix as I don't expect it to break anyone downstream for this library. My one thought would be to conditionally include it as opposed to always passing it with a value (of which one of those values could be undefined). I think this would assume less about how gatsby-source-filesystem is doing the nullish checks, and be less likely to create bugs in compatibility.

@dgrcode
Copy link
Contributor Author

dgrcode commented Mar 22, 2020

Thanks! :) I didn't say anything before, but really appreciate the work you've done with this package 👌

I agree something should be changed in gatsby-source-filesystem. I'm trying to get some discussions going on this issue: gatsbyjs/gatsby#22463

Regarding things we can do here, I'll open a PR using node mime to get the extension from the mime type provided by airtable. We can then pass the ext property based on the value returned by that library. We can continue discussing implementation details on the PR

@jbolda
Copy link
Owner

jbolda commented Apr 1, 2020

Curious as it isn't quite clear, do you know from where the ellipsis is coming. Is this something that Airtable is sending over? I have seen some reports of the cache being a problem, and part of me wonders if this is a root cause if it is using the same logic to pull from the cache.

@dgrcode
Copy link
Contributor Author

dgrcode commented Apr 3, 2020

Correct, Airtable sends an ellipsized filename when the name of the uploaded file is longer than a threshold. Not sure what that threshold is or why they decided to remove the extension from the filename.

Do you have an issue at hand that reports cache being a problem? I can have a look at their problems and tell you if I've seen the same issues due to the ellipsis.

Regarding this issue, I'm happy if you want to close it since #172 was merged. Also happy if you want to keep it open to track the potential relation with the cache issues

@github-actions
Copy link

github-actions bot commented May 4, 2020

This issue has been automatically marked as stale because it has not had any recent activity for 30 days. It will be closed if no further activity occurs within 7 days. Remove stale label, comment, and/or apply "ongoing issue" label to prevent this from being closed. Thank you for your contributions.

@github-actions github-actions bot added the stale Issue is stale. Closes automatically without activity. Apply "ongoing issue" label to prevent close. label May 4, 2020
@dgrcode
Copy link
Contributor Author

dgrcode commented May 7, 2020

I'll close this issue. Fixed by #172

@dgrcode dgrcode closed this as completed May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issue is stale. Closes automatically without activity. Apply "ongoing issue" label to prevent close.
Projects
None yet
Development

No branches or pull requests

2 participants