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] Incorrect type definition for createFileNodeForAsset #35363

Closed
2 tasks done
Joe-Edwards opened this issue Apr 7, 2022 · 8 comments · Fixed by #35422
Closed
2 tasks done
Labels
good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem) topic: TypeScript Issues and PRs related to TS in general, public typings or gatsby-plugin-typescript type: bug An issue or pull request relating to a bug in Gatsby

Comments

@Joe-Edwards
Copy link

Joe-Edwards commented Apr 7, 2022

Preliminary Checks

Description

The type definition for createRemoteFileNode (specifically the CreateRemoteFileNodeArgs interface) doesn't match the documentation or implementation, in particular:

  • store is mandatory in the types, but not present in the docs or implementation
  • cache is mandatory in the types, but not present in the docs, and optional in the implementation
  • getCache is missing from the types, but mandatory in the docs, and optional in the implementation (but you must pass one of cache or getCache)
  • reporter is mandatory in the types, but not present in the docs or implementation

Steps to Reproduce

  1. Use Typescript gatsby-node.ts
  2. Use the createRemoteFileNode function as documented:
  const fileNode = createRemoteFileNode({
    url: encodeURI(node.url),
    getCache, // Object literal may only specify known properties, and 'getCache' does not exist in type 'CreateRemoteFileNodeArgs'.
    createNode,
    createNodeId,
    parentNodeId: node.id,
  });

Expected Result

Typescript compiles successfully

Actual Result

Type errors being reported due to incorrect arguments

Environment

System:
    OS: Windows 10 10.0.19044
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 16.14.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.16.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 8.6.0 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    gatsby: ^4.10.3 => 4.10.3 
    gatsby-plugin-eslint: ^4.0.2 => 4.0.2 
    gatsby-plugin-image: 2.10.1 => 2.10.1 
    gatsby-plugin-react-helmet-async: ^1.2.1 => 1.2.1 
    gatsby-plugin-styled-components: ^5.10.0 => 5.10.0 
    gatsby-source-contentstack: 4.0.1 => 4.0.1 
    gatsby-source-filesystem: ^4.10.1 => 4.10.1 
    gatsby-transformer-json: ^4.10.0 => 4.10.0

Config Flags

No response

@Joe-Edwards Joe-Edwards added the type: bug An issue or pull request relating to a bug in Gatsby label Apr 7, 2022
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Apr 7, 2022
@LekoArts LekoArts added help wanted Issue with a clear description that the community can help with. good first issue Issue that doesn't require previous experience with Gatsby topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem) topic: TypeScript Issues and PRs related to TS in general, public typings or gatsby-plugin-typescript and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Apr 8, 2022
@LekoArts
Copy link
Contributor

LekoArts commented Apr 8, 2022

Hi, thanks for the issue!

Indeed, the TypeScript definition diverged from what is actually correct now.

The TypeScript definitions are here:

export interface CreateRemoteFileNodeArgs {
url: string
store: Store
// TODO: use GatsbyCache type (requires gatsby@2.22.13)
cache: NodePluginArgs["cache"]
createNode: Function
createNodeId: Function
parentNodeId?: string
auth?: {
htaccess_user: string
htaccess_pass: string
}
httpHeaders?: object
ext?: string
name?: string
reporter: object
}

A reference for what it actually should be is https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-filesystem/src/create-remote-file-node.js#L105 and

/**
* @typedef {CreateRemoteFileNodePayload}
* @typedef {Object}
* @description Create Remote File Node Payload
*
* @param {String} options.url
* @param {GatsbyCache} options.cache
* @param {Function} options.createNode
* @param {Function} options.getCache
* @param {Auth} [options.auth]
* @param {Reporter} [options.reporter]
*/

@Apprentice76
Copy link
Contributor

Hi, can I work on this? If I understand right, I have to modify the index.d.ts to match the references, like adding the missing argument and setting the others to optional when required.

@darylwright
Copy link

I looked into it a bit yesterday, although I don't think I'm going to implement this change. It seems that the index.d.ts will have to change. One thing that's bothering me is that the reporter field is defined in the CreateRemoteFileNodePayload type, but it isn't used when calling createRemoteFileNode(). Could it simply be omitted? Not sure what its purpose is. Here's what I believe the payload object type should look like so far:

export interface CreateRemoteFileNodePayload {
  url: string
  cache?: GatsbyCache
  getCache?: Function
  createNode: Function
  createNodeId: Function
  parentNodeId?: string
  auth?: {
    htaccess_user: string
    htaccess_pass: string
  }
  httpHeaders?: object // could also be `{ [key: string]: string }`?
  ext?: string
  name?: string
}

@Apprentice76
Copy link
Contributor

Apprentice76 commented Apr 13, 2022

The same thing as been bothering me but the function description here

/**
* @typedef {CreateRemoteFileNodePayload}
* @typedef {Object}
* @description Create Remote File Node Payload
*
* @param {String} options.url
* @param {GatsbyCache} options.cache
* @param {Function} options.createNode
* @param {Function} options.getCache
* @param {Auth} [options.auth]
* @param {Reporter} [options.reporter]
*/

has me confused as to whether the reporter param is required or not, should I remove this line as well?

I'm removing the Store param as it's not mentioned in the refrences.

Also I noticed, that createFileNodeFromBuffer has the same issue, should I change that as well? Here

export interface CreateFileNodeFromBufferArgs {
buffer: Buffer
store: Store
// TODO: use GatsbyCache type (requires gatsby@2.22.13)
cache: NodePluginArgs["cache"]
createNode: Function
createNodeId: Function
parentNodeId?: string
hash?: string
ext?: string
name?: string
}

when it should be like

module.exports = ({
buffer,
hash,
cache,
createNode,
getCache,
parentNodeId = null,
createNodeId,
ext,
name,
}) => {
// validation of the input

If someone would confirm this, I'll push the changes.

@github-actions
Copy link

github-actions bot commented May 4, 2022

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label May 4, 2022
@Joe-Edwards
Copy link
Author

This is still an issue, looks like the PR #35422 has got stuck

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label May 4, 2022
@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label May 25, 2022
@LekoArts LekoArts removed the stale? Issue that may be closed soon due to the original author not responding any more. label May 25, 2022
@raaz001

This comment was marked as off-topic.

gatsbybot pushed a commit that referenced this issue Jun 1, 2022
…5422)

* fixes issue #35363 and CreateFileNodeFromBufferArgs

* modified tests for remote-file-node and file-node-from-buffer

* updated function calls in dependencies

* removed unused typescript imports

Co-authored-by: Ward Peeters <ward@coding-tech.com>
Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
mwfrost pushed a commit to mwfrost/gatsby that referenced this issue Apr 20, 2023
…tsbyjs#35422)

* fixes issue gatsbyjs#35363 and CreateFileNodeFromBufferArgs

* modified tests for remote-file-node and file-node-from-buffer

* updated function calls in dependencies

* removed unused typescript imports

Co-authored-by: Ward Peeters <ward@coding-tech.com>
Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem) topic: TypeScript Issues and PRs related to TS in general, public typings or gatsby-plugin-typescript type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants