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

html/template: add support for JavaScript modules #31327

Closed
tomuta opened this issue Apr 7, 2019 · 8 comments
Closed

html/template: add support for JavaScript modules #31327

tomuta opened this issue Apr 7, 2019 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal Proposal-Accepted
Milestone

Comments

@tomuta
Copy link

tomuta commented Apr 7, 2019

template/html does not properly treat javascript code as javascript when using a <script type="module" tag:

<!doctype html>
<html lang="en">
	<head>
		<meta charset="utf-8" />
	</head>
	<body>
	<script type='module'>
		1 <= 2
	</script>
</html>

What did you expect to see?

<!doctype html>
<html lang="en">
	<head>
		<meta charset="utf-8" />
	</head>
	<body>
	<script type='module'>
		1 <= 2
	</script>
</html>

What did you see instead?

<!doctype html>
<html lang="en">
	<head>
		<meta charset="utf-8" />
	</head>
	<body>
	<script type='module'>
		1 &lt;= 2
	</script>
</html>

See also: https://www.w3.org/TR/html5/semantics-scripting.html#element-attrdef-script-type

I created a pull request that fixes the issue: #27697

@ALTree ALTree added this to the Go1.13 milestone Apr 9, 2019
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 9, 2019
@empijei
Copy link
Contributor

empijei commented Apr 16, 2019

So, a couple of thoughts on this: module context should probably follow the same escaping rules that scripts currently have.

The only difference is for import statements. Those will likely need to sanitize the string as script src AND javascript string.

@mikesamuel do you have more context on JS modules differences with normal scripts?

@mikesamuel
Copy link
Contributor

@empijei There's no significant difference.
A JS ModuleBody is just a strict mode ScriptBody that can also contain import and export declarations.
So I think we can add "module" to https://golang.org/src/html/template/js.go#L390

func isJSType(mimeType string) bool {
        ...
	switch mimeType {
	case
		"application/ecmascript",
                ...
        }
        ...
}

@tomuta
Copy link
Author

tomuta commented Apr 21, 2019

The only reason I created this issue is that I was asked to create it in addition to the merge request I already created: #27697. And yes, it does add a single line to this function.

@mikesamuel
Copy link
Contributor

@tomuta, thanks for explaining. Now I see https://go-review.googlesource.com/c/go/+/135417

@mikesamuel
Copy link
Contributor

@empijei

The only difference is for import statements. Those will likely need to sanitize the string as script src AND javascript string.

This is a good point, and there're module expressions: import(...) which is only stage 3 as far as TC39 is concerned but which is widely implemented: https://caniuse.com/#feat=es6-module-dynamic-import

@odeke-em odeke-em changed the title html/template: Add support for JavaScript modules proposal: html/template: add support for JavaScript modules Apr 24, 2019
@rsc
Copy link
Contributor

rsc commented Apr 24, 2019

It sounds like we should do this (and accept CL 135417 once it is reviewed).

@rsc rsc changed the title proposal: html/template: add support for JavaScript modules html/template: add support for JavaScript modules Apr 24, 2019
@gopherbot
Copy link

Change https://golang.org/cl/135417 mentions this issue: html/template: add support for JavaScript modules.

@gopherbot
Copy link

Change https://golang.org/cl/175218 mentions this issue: html/template: add support for JavaScript modules

@golang golang locked and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal Proposal-Accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants