-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
JS Linked Lists: Add mention of ES6 modules with Node #27101
JS Linked Lists: Add mention of ES6 modules with Node #27101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we've largely tried to move away from linking to outside articles inline, I wonder if the linked resource would make sense in the additional resources section.
Also: I wonder if this addition makes the most sense in this lesson. Part of me thinks it should go in the es6 modules lesson, but the other part feels like that's to early. WDYT?
@wise-king-sullyman My feeling is that the ESM lesson might be a little early. My understanding is that the ESM lesson is focused not just on what ESM is generally but primarily in the context of being used in a browser, hence the justification for webpack to bundle. Hence the suggestion to add it where the there's the first likely instance of its relevance, in the Linked List project. I don't think many if any people would find a significant need to use multiple modules in the recursion project. About the link/additional resources, given that the proposed addition is in the Linked List project itself which doesn't have an additional resources section (don't think any projects do?), I'm not sure if separating the link out from the note will be better? That's going by having the addition in the project content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Thank you for your contribution! I appreciate the effort you've put into this commit. The code looks clean and well-organized. Keep up the good work!"
@MaoShizhong Hmm, I've been thinking about this a lot, and while yes the ESM lesson is more in the context of running in the browser it does also touch on NPM and the package file which ties into your note quite nicely IMO. To me it feels like a much better fit than having it in a project. I do understand wanting to add it where it will be most relevant though. What would you think about adding the actual info and linking to the CJS vs ESM blogpost in the ESM lesson and having a note that calls back to it in the linked list project? |
@wise-king-sullyman Going through the ESM lesson again, what you're saying does make sense. I'm a little unsure exactly how I'd want to integrate the same content in the current rendition of the ESM lesson in terms of the flow, keeping it relevant in context, whilst also not risking overwhelming the learner in what's already commonly an overwhelming section (which I also think is in part related to the current structure). |
@MaoShizhong agree it would be a lot easier to fit this into the ESM lesson after that rework, I don't think it would be awful as is though, and I think it would make sense to just add it to the end of the If you have a better idea than that though I'm not hard committed to it. |
Linked List Node/ESM mention amended to account for new earlier introduction.
Almost all linting errors have been fixed. The remaining ones are ones I'm not sure how to resolve as there are no rules set for them (missing code block language for blocks that do not require a language, and links that link to a page section instead of an external link). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
Thanks for all your work on this! |
Because
Currently, Node uses CommonJS by default and needs to be told in some way to use ESM instead.
Whilst this is simple, it's not made clear in the curriculum so far and so sometimes people wish to use multiple files in these comp sci projects but encounter issues with Node not recognising ESM by default.
This should add a small instruction for how to tell Node to use ESM and a little link that introduces the situation a little further.
This PR
Issue
Closes #26679
Additional Information
Latest Node (non-LTS) has added experimental support for automatic module-type recognition which will be interesting in the future.
Pull Request Requirements
location of change: brief description of change
format, e.g.Intro to HTML and CSS lesson: Fix link text
Because
section summarizes the reason for this PRThis PR
section has a bullet point list describing the changes in this PRIssue
section