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

Webpack: New second lesson after Project Restaurant Page #27962

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

MaoShizhong
Copy link
Contributor

@MaoShizhong MaoShizhong commented May 11, 2024

Because

The current Webpack lesson is a known pain point in the curriculum journey, primarily due to the sudden increase in complexity and steps of tooling, coupled with how much of the learning is left to external resources. These external resources limit the cohesiveness of teaching material, and can prove troublesome in the learning process when each resource includes a few unnecessary tidbits that can easily throw learners off (such as multiple entry points in the webpack official guides when learning about html-webpack-plugin). They also are frequently aimed at people who may know a little more about bundlers already, or are following a different course/tutorial. Thus, things often feel disjointed with the TOP curriculum.

While improving the Webpack lesson itself is the primary focus, it was also identified that the preceding ES6 modules lesson is fairly coupled with it, as Webpack and bundlers are introduced there instead of the Webpack lesson.

The order of the ES6 modules lesson was also identified as a little odd, and the order and flow could be improved by focusing on ES6 modules as a primary topic straight away, then introducing bundlers and Webpack in their own separate lesson.

General plan:

  • Rewrite ES6 modules lesson
  • Rewrite Webpack lesson
  • Write new "Webpack revisited" lesson to go immediately after "Project: Restaurant Page".
    • Move more advanced but "convenience" topics there, like webpack-merge
    • Extract template repos section from "Linting" and place it here instead
  • Tweak Project: Restaurant Page to reference any new material from preceding lesson changes if necessary

This PR

  • Adds new Webpack lesson for after the Restaurant Page project (latest "Revisiting Webpack" live preview)
  • Removes section and other mentions of Template Repos from the current Linting lesson, as that contents is now covered in this new lesson
  • Adds assignment section to Linting lesson to follow style guide about lesson structure (latest "Linting" live preview)
    • Adjustments made to section content to accommodate for the change in structure

Issue

Related to #26976
Related to #26977
(closed by #27953)

Additional Information

This will be something to merge alongside 3 other PRs:

  1. (Webpack: Rewrite lesson with in-house tutorial (alongside ES6 Modules rewrite) #27961)
  2. (ES6 Modules: Rewrite lesson (alongside separate PR for Webpack rewrite) #27953)
  3. (Feature: Add new lessons from ESM/Webpack restructure to both pathways theodinproject#4522)

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

Forgot that the first webpack lesson rewrite has a round up section
which this was basically a duplicate of. The exact contents better suits
how it's placed in the first lesson, so this lesson's one is to be removed.
@MaoShizhong
Copy link
Contributor Author

Question for any reviewers: What's your opinion on also mentioning mini-css-extract-plugin in this lesson?

Considerations:

  • Rewritten Webpack tutorial does not include this, only css-loader -> style-loader then import CSS into JS. I feel including mini-css-extract-plugin will probably be too much there given everything else already going on, but it can be placed there if appropriate. I just feel mini-css-extract-plugin requires explanation as to the whole point.
  • Leaving things as is will have people potentially run into flashes of unstyled content, since the JS must process before it can load the CSS (since the CSS is within the JS as a string, not a separate .css file). mini-css-extract-plugin prevents this.
  • Will require having to explain why we import CSS into JS just to extract it back out again, as opposed to just linking the CSS file in the HTML template, then using html-loader only (no style-loader, no css-loader and no importing CSS).
    • Will have to mention this is how build tools do things under the hood, as more complex projects might require more modular CSS approaches where you import separate CSS files to individual JS modules. Therefore, we import CSS into JS for the dev experience, then use mini-extract-css-plugin to extract the CSS back out to files for production.

Right now, I feel it's valuable to mention it here after npm scripts and before webpack modes.
Like any other setup, template repos make this a doddle in the future, and I think it makes sense to make learners aware of this concept, as opposed to facing FOUC, not knowing why, then also not knowing why Vite doesn't do the same thing when they get to React (because it's extracting imported CSS under the hood for you).

@xandora
Copy link
Member

xandora commented May 30, 2024

Dang... that's a lot of words. 👀

Follows lesson file structure as prescribed by our style guide.
Some resources from the main contents have been moved to assigned reading.

Removed "JavaScript Linters" resource due to duplicate information, and
it also makes an explicit recommendation that differs from our lesson
contents, which can cause confusion.
More consistent with "Linting" section, as opposed to "ESLint" section.
Prettier installation guide added to be more consistent with ESLint links
in the Linting section.
Knowledge check questions amended to account for this change.
Capitalisation nits.
Fixed descriptive link text for ESLint
@MaoShizhong
Copy link
Contributor Author

Added ### Assignment section to the Linting lesson here since this PR moves a whole section out of it.

Some minor section content changes have also been made to accommodate the structure change.

GitHub outage leading to stalled workflows which were aborted.
Unable to manually trigger otherwise.
MaoShizhong and others added 2 commits June 30, 2024 00:07
It's both a relevant tip for the lesson, and it helps reduce confusion
caused by moving the template repos section to a lesson more than one
lesson back (in case someone was at, say, the To-do list project when
these changes get merged).
Co-authored-by: Jack Bogart <[email protected]>
@MaoShizhong MaoShizhong added the Status: Do Not Merge This PR should not be merged until further notice label Jun 30, 2024
Use more assertive wording
@MaoShizhong
Copy link
Contributor Author

Regarding mini-css-extract-plugin, I'm now opting to not add it to any of the new Webpack PRs.

The original idea was that it would help explain why we import CSS instead of just linking the stylesheet in the HTML template and using html-loader, by sharing how more complex projects might want to split CSS files and import them only where needed. mini-css-extract-plugin would prevent FOUCs since it'd extract and link files, rather than style-loader just loading a string in the JS bundle.

Not really necessary here, I think. Opens too many doors to rabbit holes (e.g. to combine multiple CSS file imports into 1, you'd need to set up a bunch of optimisation properties in the config too). I think all we'd need is the rewritten Webpack tutorial to just mention "most build tools do a little more than this, but this is enough to allow us to get used to importing CSS files in your JS", since that'll be more important in the React course with Vite, where Vite handles all the CSS loading for you.

End waffle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: JavaScript Involves the JavaScript course Status: Do Not Merge This PR should not be merged until further notice Type: New Lesson Involves a new lesson or lessons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants