-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[WIP] Introduce HTML Modules #4505
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.
Exciting, couple early comments I found while skimming.
source
Outdated
<ol> | ||
<li><p>Create a new <span>Document</span> node <var>document</var> whose | ||
<span data-x="concept-document-content-type">content type</span> is | ||
<code data-x="">text/html</code> and mark it as being an <span |
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'd suggest this is a TODO as well, as I hope we can agree it should match the MIME type we end up picking.
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.
Yes, updated in latest commit.
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 the early feedback!
source
Outdated
<span data-x="concept-document-content-type">content type</span> is | ||
<code data-x="">text/html</code> and mark it as being an <span | ||
data-x="HTML documents">HTML document</span>. Let <var>document</var> be in | ||
<span>no-quirks mode</span>. <var>document</var> must be considered an |
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.
"no-quirks" is the default so the main thing we have to ensure is that the parser cannot set it.
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.
Removed the redundant no-quirks assigment here at HTML Module doc creation time.
See the initial insertion mode for the parser change that prevents the parser from switching it to quirks. I just followed the existing pattern for iframe srcdoc there.
source
Outdated
<code data-x="">text/html</code> and mark it as being an <span | ||
data-x="HTML documents">HTML document</span>. Let <var>document</var> be in | ||
<span>no-quirks mode</span>. <var>document</var> must be considered an | ||
<span>HTML module document</span>.</p></li> |
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.
Should we set the origin as well?
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.
Yes; updated this step to assign the origin from the environent settings object retrieved from the HTML Module script.
source
Outdated
<ol> | ||
<li><p>Set <var>scriptEntry</var>.[[InlineModuleRecord]] = null.</p></li> | ||
|
||
<li><p>Set <var>scriptEntry</var>.[[ExternalScriptURL]] = *script’s* src URL.</p></li> |
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.
This needs more formal wording.
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.
Updated in latest commit.
source
Outdated
<p>Otherwise:</p> | ||
|
||
<ol> | ||
<li><p>Set <var>scriptEntry</var>.[[InlineModuleRecord]] = null.</p></li> |
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.
You cannot use =
like that. Seems like you want "to". (This happens more often, both above and below.)
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.
Updated in latest commit.
source
Outdated
@@ -58347,6 +58354,10 @@ o............A....e | |||
|
|||
</li> | |||
|
|||
<li><p>If the element's <span>node document</span> is an <span>HTML module document</span>, then |
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.
Can script tags be dynamically added to an HTML module's document? If so, I don't understand when those scripts would be run.
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.
With the current design, dynamically added scripts won't execute (as if they were added to a dynamic document).
For non-dynamically-added scripts: after the HTML Module document is parsed, we basically take a snapshot of it at that time and the <script>
elements at that point become the HTML module's child modules in the module graph. Fiddling dynamically with the HTML Module doc's tree after that point won't affect the structure of the module graph, and won't affect which scripts are executed or the order of execution.
I split the non-normative part of this line into a separate note and modified it to try to clarify what I meant here.
I'm open to creating an issue on the whatwg/html repo if you think this is contentious or needs to be open to wider discussion. Our design philosophy so far has generally been to treat the HTML module document as an inert dynamic document except for the special bits needed to enable HTML modules to work as a non-leaf node in the module graph.
Thanks for the feedback!
@dandclark could you clarify why you closed this? |
@annevk I didn't meant to close it. I was doing something else in my fork of the whatwg repo and moved the source branch for these changes, forgetting that doing so would affect this PR, which ended up getting auto-closed due to my actions. I've restored the changes and reopened the PR. Sorry for the noise! I've also taken this opportunity to resolve the merge conflicts with the introduction of JSON modules. |
fyi, maybe it happened again. |
I cannot reopen it since we have renamed the default branch after 3 Oct 2020 and because this pull request has been closed since then it did not get automatically adjusted. It's probably best to open a new pull request at this point @dandclark. |
We would love to see some action on this. Since HTML imports has been obsoleted, we desperately need to fill the gaps with purely declarative modular Web Components. |
To echo the above comments, I too feel this would greatly aid Web Component adoption. Should this ticket PR be reopened or have we abandoned the proposal? |
Since folks are not reading earlier comments I'll lock this. |
Introduce HTML Modules, an extension of the ES modules system that enables importing of HTML content
This is not yet ready to merge in given there are still open questions about the design, e.g. whether HTML Modules require a new MIME type. The intent here is to facilitate discussion and tease out issues that can be better found when making changes against the actual spec, with the intention of eventually merging in the updated PR once consensus has been reached on all open issues. These issues are primarily being tracked over in the w3c/webcomponents repo.
There is a corresponding PR against the ES spec here, which refactors InnerModuleInstantiation/InnerModuleEvaluation to avoid issues with HTML Module Record's different way of tracking its requested modules.
I've placed the built result of these changes here: https://dandclark.github.io/built-specs/whatwg-html/html-modules.html
/acknowledgements.html ( diff )
/infrastructure.html ( diff )
/origin.html ( diff )
/parsing.html ( diff )
/scripting.html ( diff )
/webappapis.html ( diff )