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

Should node id's be unique and meaningless? #23638

Closed
ediblecode opened this issue Apr 30, 2020 · 7 comments
Closed

Should node id's be unique and meaningless? #23638

ediblecode opened this issue Apr 30, 2020 · 7 comments
Labels
stale? Issue that may be closed soon due to the original author not responding any more. type: documentation An issue or pull request for improving or updating Gatsby's documentation

Comments

@ediblecode
Copy link
Contributor

Summary

I'm writing a source plugin where I download JSON from an API and create nodes - all pretty standard. Objects I receive have an a field called topicId with is a guid (uuid) that represents the main id of the item.

Now I could use this field directly as the node id in Gatsby like this:

createNode({
  id: item.topicId
})

However, the createNodeId function exists (to create guid ids) so it seems like I should use it, and store the topicid as its own field, like this:

const { topicId } = item;
createNode({
  id: createNodeId(topicId),
  topicId
})

So the question is, should id's be unique and meaningless? And therefore which approach above is favourable?

One of the benefits of createNodeId is that it 'will generate different IDs for different plugins if they use same input'. In my particular case, this is irrelevant because I've already got a guid, so I could probably safely just use it directly.

Relevant information

This comment suggests that node ids should be unique and meaningless, so people don't rely on them for anything, which makes sense.

However, I couldn't find anything about this in the docs either in node creation or createNodeId. In fact, several of the examples in the docs actually don't use createNodeId and use predictable ids directly.

I'd be happy to contribute to the docs if anyone thinks this needs adding/clarifying.

Environment (if relevant)

N/A

File contents (if changed)

gatsby-config.js: N/A
package.json: N/A
gatsby-node.js: N/A
gatsby-browser.js: N/A
gatsby-ssr.js: N/A

@ediblecode ediblecode added type: question or discussion Issue discussing or asking a question about Gatsby type: documentation An issue or pull request for improving or updating Gatsby's documentation labels Apr 30, 2020
@pieh
Copy link
Contributor

pieh commented Apr 30, 2020

ids should be unique - we have dictionary of nodes by id so if two nodes share same id - then it would just mean that one overwrites the other. Note that the this is in global scope, so even if nodes have different types - they still can't use same id

#22004 is example of issues when ids are not unique (in this case those have different types but same id).

The "meaningless" part is somewhat related to uniqueness - it's not strict requirement, but it is good practice - when users would want to query by id (and not by something like topicId) it just increases chance of colissions, because you need to factor in that nodes that you create in your plugin or site are just parts of all the nodes that exist in data layer - other plugins share the same id pool. So best way to go about it is to make id meaningless (so users don't use this) and have meaningful identifiers as some other field (topicId).

several of the examples in the docs actually don't use createNodeId and use predictable ids directly.

It does sound like the documentation adjustments is good idea, but there is always nuance. Sometimes we don't want to go on tangential problem when explaining some particular concept. So we might need to evaluate this for each documentation page separately - do you have examples in mind?

@ediblecode
Copy link
Contributor Author

Thanks for clarifying. I'll def use the second option and store topicId separately to id and use createNodeId.

I see what you mean about the nuance. Don't want to overwhelm someone!

I was thinking mostly about the Node creation page and specifically the Node relationship storage model section. It explains the flat dictionary you referenced above and the implications, and does make a reference to child field inference.

I wonder if this is a good place to explain about unique/meaningless ids?

Or possibly a new section alongside Parent/Child/Refs, Fresh/stale nodes, Changing a node's fields and Node Tracking - it definitely fits on that page as one of the fairly important things to know.

@marcysutton
Copy link
Contributor

Hey @ediblecode, thanks for opening the issue! And thanks to @pieh for the initial triage. I think a new section on the Node Creation page about handling IDs would be awesome. If you're up for contributing it, I'd say go for it!

@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 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. You can also add the label "not stale" to keep this issue open!
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 22, 2020
@ediblecode
Copy link
Contributor Author

Not stale, still going to submit a PR for the updated docs just haven't got round to it, sorry

@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 30, 2020
@LekoArts LekoArts removed the type: question or discussion Issue discussing or asking a question about Gatsby label Jun 4, 2020
@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 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. You can also add the label "not stale" to keep this issue open!
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 Jun 24, 2020
@github-actions
Copy link

github-actions bot commented Jul 5, 2020

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.
Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.
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 again for being part of the Gatsby community! 💪💜

@github-actions github-actions bot closed this as completed Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale? Issue that may be closed soon due to the original author not responding any more. type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

No branches or pull requests

4 participants