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

Library: Introduce crypto.randomUUID() instead of array index? #29341

Open
2 of 3 tasks
MaoShizhong opened this issue Jan 21, 2025 · 4 comments · May be fixed by #29463
Open
2 of 3 tasks

Library: Introduce crypto.randomUUID() instead of array index? #29341

MaoShizhong opened this issue Jan 21, 2025 · 4 comments · May be fixed by #29463
Assignees
Labels
Status: Help Wanted This issue can be assigned to other contributors

Comments

@MaoShizhong
Copy link
Contributor

Checks

Describe your suggestion

The Library project instructs to associate the Book instances in the library array with their respective DOM elements and recommends doing so via a data-* attribute. It also recommends using the instance's array index as the link.

This is of course a problem when books are deleted because either the array element is deleted/nulled but still takes space to preserve the indices of the books, or logic has to be added to amend the DOM to associate with the new indices, and this has been a rather common pain point, less from a "I don't know how to implement this" POV but more a "surely this isn't a sensible way to deal with this sort of thing?"

I feel it'd be more pedagogically appropriate to recommend giving the book objects their own unique ID and using that as the link to the DOM instead of the dynamic index. This could be via an auto-increment or possibly more appropriately, crypto.randomUUID(). No packages, all built-in and very appropriate for this feature.

The instruction step could also do with a link to documentation on data-* attributes since I believe this is the first time they're properly mentioned in the curriculum (at least in the JS pathway).

Proposal:

  1. Change use of array index as the link between DOM and book objects to some form of ID, perhaps a UUID using the built-in crypto.randomUUID().
  2. Link to data-* docs in the same instruction step.

Path

Ruby / Rails, Node / JS

Lesson Url

https://www.theodinproject.com/lessons/node-path-javascript-library

(Optional) Discord Name

No response

(Optional) Additional Comments

No response

@wise-king-sullyman
Copy link
Member

Seems like a good idea to me, thanks @MaoShizhong!

@MaoShizhong
Copy link
Contributor Author

Might want to mark this as "help wanted" so others know it's available for assignment request. If no one wants it by next week, I'll grab it.

@CouchofTomato CouchofTomato added the Status: Help Wanted This issue can be assigned to other contributors label Feb 7, 2025
@dekr1sh
Copy link
Contributor

dekr1sh commented Feb 12, 2025

I'd be happy to help if its available for grabs

@JoshDevHub
Copy link
Contributor

Thank you for volunteering @dekr1sh

I'll assign you. Let us know if you have any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Help Wanted This issue can be assigned to other contributors
Projects
None yet
5 participants