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

ES6 Modules: Add warning about opening directly in browser #28746

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

MaoShizhong
Copy link
Contributor

@MaoShizhong MaoShizhong commented Sep 5, 2024

Because

ES6 modules require a dev server to be opened (as opposed to opening directly in the browser under file://) due to CORS. I had assumed we directly assigned people to use the Live Preview extension in VSC in an earlier lesson but they're actually just tips, so not everyone would have been using it (user on Discord confused by CORS error due to opening via terminal).

A quick note can be added that live server is necessary for now. Later, webpack-dev-server will be introduced.

This PR

  • Adds warning box for opening ES6 modules, advising use of Live Preview instead

Issue

N/A

Additional Information

Changed from Live Server to Live Preview due to #28801

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

@github-actions github-actions bot added the Content: JavaScript Involves the JavaScript course label Sep 5, 2024
@damon314159
Copy link
Member

I did a quick site wide Google for cors and didn't see any mentions of cors this early in the course. If that is the case, is it wise to introduce it in a note box with no further explanation?

That feels like it would cause a lot of questions in discord, or just unnecessary rabbit holes into cors and headers and such.

@MaoShizhong
Copy link
Contributor Author

MaoShizhong commented Sep 6, 2024

I did a quick site wide Google for cors and didn't see any mentions of cors this early in the course. If that is the case, is it wise to introduce it in a note box with no further explanation?

That feels like it would cause a lot of questions in discord, or just unnecessary rabbit holes into cors and headers and such.

The note is less about CORS itself and more that to run the ESM code you actually need a server, because if you try to open the HTML file in the browser directly (as some might still be doing), they will actually get an error referring to CORS. Perhaps if I wrote "CORS" then my intention might be clearer.

That being said, looking at this again, it probably doesn't need a note box but rather just adding to the end of the previous sentence:

If you had coded along with the IIFE example at the start of the lesson, try rewriting the JavaScript to use import and export, and link only the entry point as a module script. Due to browser security reasons, ES6 modules cannot be loaded if you open the HTML file directly in the browser, so make sure you use Visual Studio Code's Live Server extension if you aren't already.

What do you think?

@damon314159
Copy link
Member

I think that's a good way to explain the situation without inviting questions that are going to be answered later on

Better served as main lesson text rather than a note box
@MaoShizhong MaoShizhong merged commit f2a77cf into TheOdinProject:main Sep 25, 2024
2 checks passed
@MaoShizhong MaoShizhong deleted the es6-modules-live-server branch September 25, 2024 23:18
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants