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

gatsby-source-filesystem Check if file exists locally before downloading #4493

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions packages/gatsby-source-filesystem/src/create-remote-file-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,23 @@ module.exports = ({ url, store, cache, createNode, auth = {} }) => {
digest + path.parse(url).ext
)

const createFileSystemNode = () => {
// Create the file node and return.
createFileNode(filename, {}).then(fileNode => {
// Override the default plugin as gatsby-source-filesystem needs to
// be the owner of File nodes or there'll be conflicts if any other
// File nodes are created through normal usages of
// gatsby-source-filesystem.
createNode(fileNode, { name: `gatsby-source-filesystem` })
resolve(fileNode)
})
}

if (fs.existsSync(filename)) {
createFileSystemNode()
return
}

// Fetch the file.
let statusCode
let responseHeaders
Expand Down Expand Up @@ -100,15 +117,7 @@ module.exports = ({ url, store, cache, createNode, auth = {} }) => {
fs.removeSync(tmpFilename)
}

// Create the file node and return.
createFileNode(filename, {}).then(fileNode => {
// Override the default plugin as gatsby-source-filesystem needs to
// be the owner of File nodes or there'll be conflicts if any other
// File nodes are created through normal usages of
// gatsby-source-filesystem.
createNode(fileNode, { name: `gatsby-source-filesystem` })
resolve(fileNode)
})
createFileSystemNode()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you not create a new function please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it, how would you prefer it to be handled?
The code needs to createFileNode whether or not it has to download the remote file, and originally that function was being called on responseStream.on(end)'.
Would you rather I just write out the function twice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I'm not sure what this PR is fixing now. Files aren't redownloaded — a 304 response has an empty body.

})
}))
}