Skip to content
This repository was archived by the owner on Jul 13, 2020. It is now read-only.

adding support for script defer, improving the resolution process for data-esprima-src#22

Merged
guybedford merged 2 commits intoModuleLoader:masterfrom
caridy:defer
Aug 15, 2013
Merged

adding support for script defer, improving the resolution process for data-esprima-src#22
guybedford merged 2 commits intoModuleLoader:masterfrom
caridy:defer

Conversation

@caridy
Copy link
Copy Markdown
Contributor

@caridy caridy commented Aug 9, 2013

If injecting es6-module-loader dynamically or using defer on the script tag, the resolution of esprima src is going to be a little bit more complex. Aside from that, moving the esprimaSrc computation into loadEsprima method makes more sense in preparation for forking the logic if the browser supports es6 modules.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

breaking if data-esprima-src is found in any of the current script tags, otherwise use the last script tag (assuming it is the es6-module-loader which is normally blocking the page rendering).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a way to work around this breakage? data-esprima-src is used to, in-part, resolve #13

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if you don't want to have a break in the cycle, you will have to walk all scripts in search for data-esprima-src, which is just fine, in the majority of the cases it will be few scripts, but in some edge cases we might have a lot of script tags on the page at the moment we inject the es6-module-loader (which could be inserted at the top or at the bottom btw). I didn't wanted to change too much of the code style, but I can definitely do some adjustments, specially to avoid calling getAttribute() twice, and the break. Will update the PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

About #13, I really don't like the global variable to set the location of esprima, I rather prefer to have a better way to detect it from the scripts in the page, considering that at least one script is going to be there anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

with this approach, it will walk the array, but do not necessary touch all elements, which is a nice optimization. under ideal circumstances, we will touch the DOM once, to collect the data-esprima-src, or fallback to collect the src.

@caridy
Copy link
Copy Markdown
Contributor Author

caridy commented Aug 14, 2013

Any update on this?

@addyosmani
Copy link
Copy Markdown
Member

I would love to get some feedback from @guybedford before we do anything further with this pull request.

guybedford added a commit that referenced this pull request Aug 15, 2013
adding support for script defer, improving the resolution process for `data-esprima-src`
@guybedford guybedford merged commit f2f4164 into ModuleLoader:master Aug 15, 2013
@guybedford
Copy link
Copy Markdown
Member

Looks great to me... we need to document this data-esprima-src behaviour properly as well... will include a note in the readme shortly.

@caridy
Copy link
Copy Markdown
Contributor Author

caridy commented Aug 16, 2013

thanks @guybedford

@caridy caridy deleted the defer branch August 16, 2013 19:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants