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

bpo-31715 Add mimetype for extension .mjs #3908

Merged
merged 7 commits into from
Oct 8, 2018

Conversation

bmeck
Copy link
Contributor

@bmeck bmeck commented Oct 6, 2017

@bmeck bmeck requested a review from a team as a code owner October 6, 2017 15:27
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@MylesBorins
Copy link

The CLA has now been signed, is there anything blocking this?

@Mariatta
Copy link
Member

Needs review from @python/email-team and news entry.

@MylesBorins
Copy link

MylesBorins commented Aug 15, 2018

Thanks @Mariatta

I've send @bmeck a PR that includes the news entry in bmeck@f54e12c

Does it look like I did that correctly?

@Mariatta
Copy link
Member

@MylesBorins The news entry lgtm. If @bmeck ended up accepting your PR, then you should sign Python's CLA too.

And we'll need to make sure to add Co-authored by: in the commit message of this PR. (The committer will do this).

@MylesBorins
Copy link

thanks @Mariatta I've privately communicated with @bmeck to coordinate... signed the CLA... and look forward to the review.

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@MylesBorins
Copy link

CLA is confirmed if you want to re-run commit bot

@MylesBorins
Copy link

here's an example of this biting people in the wild

https://twitter.com/kuvos/status/1046984084575150082

Anyway we could get review on this?

@emilyemorehouse
Copy link
Member

I'll leave the argument to someone more familiar with RFC details on mimetypes (there are 3+ RFCs on this), but I think we should maintain consistency between our support of .js and .mjs files.

Currently, .js files are application/javascript, but this proposes to add .mjs as text/javascript, which is seemingly obsolete for modern browsers.

@MylesBorins or @bmeck, do you see a reason to support text/javascript instead of application/javascript?

@bmeck
Copy link
Contributor Author

bmeck commented Oct 2, 2018

@emilyemorehouse per the up to date HTML standard text/javascript is the proper response to send to browsers https://html.spec.whatwg.org/multipage/scripting.html#scriptingLanguages:javascript-mime-type

@emilyemorehouse
Copy link
Member

Sounds good, I'd make the argument that both should use text/javascript then. I'll keep an eye on this if you don't get a response from the email team code owners soon.

@MylesBorins
Copy link

I've opened #9678 and https://bugs.python.org/issue34875 to update the mimetype of .js

Lib/mimetypes.py Outdated
@@ -450,6 +450,7 @@ def _default_mime_types():
'.mht' : 'message/rfc822',
'.mhtml' : 'message/rfc822',
'.mif' : 'application/x-mif',
'.mjs' : 'text/javascript',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be application/javascript for consistency with .js

Copy link

@mathiasbynens mathiasbynens Oct 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

text/javascript is correct, and .js should be changed to use it as well. Please see the discussion above. #3908 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asveltov's rationale for application/javascript is explained here: https://bugs.python.org/issue34875

Copy link

@mathiasbynens mathiasbynens Oct 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be open to adding .mjs support with application/javascript until https://datatracker.ietf.org/doc/draft-ietf-dispatch-javascript-mjs/ gets officially accepted? It would be better to have some support than to have none at all, given that people are already running into issues with SimpleHTTPServer.

Note that Google Chrome already explicitly supports mjs: https://chromium-review.googlesource.com/c/chromium/src/+/1117070 (with text/javascript)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reading this, it sounds like you are open to that. That’s great! I look forward to Python supporting mjs, and eventually serving it as text/javascript.

@bmeck
Copy link
Contributor Author

bmeck commented Oct 8, 2018

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@asvetlov: please review the changes made to this pull request.

Add mimetype for file extension ``.mjs``. This will allow ``python -m`` to
support applications attempting to load ECMAScript Modules with the ``.mjs``
extension.
Assiciate ``.mjs`` file extension with ``application/javascript`` MIME Type.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Associate

@asvetlov asvetlov merged commit 0854b92 into python:master Oct 8, 2018
@bedevere-bot
Copy link

@asvetlov: Please replace # with GH- in the commit message next time. Thanks!

MylesBorins pushed a commit to MylesBorins/cpython that referenced this pull request Dec 6, 2018
MylesBorins pushed a commit to MylesBorins/cpython that referenced this pull request Dec 6, 2018
MylesBorins pushed a commit to MylesBorins/cpython that referenced this pull request Dec 6, 2018
MylesBorins pushed a commit to MylesBorins/cpython that referenced this pull request Dec 6, 2018
ned-deily pushed a commit that referenced this pull request Dec 20, 2018
ned-deily pushed a commit that referenced this pull request Dec 20, 2018
ned-deily pushed a commit that referenced this pull request Dec 20, 2018
ned-deily pushed a commit to ned-deily/cpython that referenced this pull request Dec 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.