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

Refactor importHref #4209

Closed

Conversation

gronke
Copy link
Contributor

@gronke gronke commented Dec 9, 2016

  • uses altering test assets to ensure tests do not affect each other
  • ensure importHref always returns a link element
  • ensure onload() and onerror() functions are only called once when the same element was loaded again

Blocked by webcomponents/webcomponentsjs#655 because the load event is invoked twice when a failed import attempt is repeated.

tomalec and others added 3 commits November 11, 2016 17:57
Seems to be a typo, that break markdown formatting.
Remove dependency on CustomElements for IE detection
@gronke
Copy link
Contributor Author

gronke commented Dec 9, 2016

@cdata 🎉 would you please have a look at this changes that were build on top of your pull-request #4200 to improve importHref.

link = document.createElement('link');
link.rel = 'import';
link.href = href;
link.__loaded = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think __loaded is already used by the polyfill, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true 😳

// In case of an error, remove the link from the document so that it
// will be automatically created again the next time `importHref` is
// called.
document.head.removeChild(link);
link.__loaded = true;
link.__error = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

The intention of the change is to discard this link. What is the benefit of tracking state on the link if it is about to be discarded?

gronke added a commit to gronke/polymer that referenced this pull request Dec 9, 2016
gronke added a commit to gronke/polymer that referenced this pull request Dec 9, 2016
throw new Error('Load handler should not be called.');
}, function() {
b = Polymer.Base.importHref('does_not_exist.html', function() {
throw new Error('Load handler should still not be called.');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless webcomponents/webcomponentsjs#655 is merged this test fill fail on browsers without native html imports support (e.g. Firefox on Travis CI).

dfreedm and others added 19 commits December 14, 2016 10:34
Close backtick in ISSUE_TEMPLATE.md
…olymer#4262.

* `Polymer.Settings.suppressTemplateNotifications `- disables `dom-change` and `rendered-item-count` events from `dom-if`, `dom-repeat`, and `don-bind`. Users can opt back into `dom-change` events by setting the `notify-dom-change` attribute (`notifyDomChange: true` property) to `dom-if`/`don-repeat` instances.
* `Polymer.Settings.suppressBindingNotifications` - disables notify effects when propagating data downward via bindings. Generally these are never useful unless users are explicitly doing something like `<my-el foo="{{foo}} on-foo-changed="{{handleFoo}}">` or calling `addEventListener('foo-changed', ...)` on an element where `foo` is bound (we attempted to make this the default some time back but needed to revert it when we found via Polymer#3077 that users were indeed doing this).  Users that avoid these patterns can enjoy the potentially significant benefit of suppressing unnecessary events during downward data flow by opting into this flag.
Fix `strip-whitespace` for nested templates.
…ents

Add global flags to suppress unnecessary notification events. Fixes Polymer#4262
… unroll behavior lifecycle calls, (3) avoid creating a custom constructor when not used, (4) provide `_skipDefineProperty` setting on behaviors which copies properties via assignment rather than `copyOwnProperty`
kevinpschaaf and others added 10 commits February 16, 2017 18:22
Can't rely on `__proto__` on IE10, but that browser doesn't need to avoid `properties`.
… dom targeted css build and native custom properties, will copy styles into the Shadow DOM template rather than collapsing them into a single style. This will (1) allow the browser to optimize parsing of shared styles because they remain intact, (2) reduce the size of the css build resources when shared styles are used since they are not pre-collapsed. This option does perform registration runtime work to add included styles to element templates.
Read properties off of proto during configuration.
Exclude SD polyfill tests for Edge due to lack of workarounds for Edg…
change lastresponse to last-response in dom-bind example
dfreedm and others added 11 commits February 27, 2017 14:13
Previously, importHref would cache failed results. This caused failures
to be cached indefinitely, even when the failure was the result of
conditions that are subject to change over time (such as poor network
connectivity).

This implementation no longer caches failures. Additionally, it relies
on the link tag's presence in the <head> of the document as an
indication that it is loading or has loaded, rather than keeping an
object map of URLs to link tags.
@gronke gronke force-pushed the enhance-refactor-import-href branch from 636c858 to 163f4c0 Compare February 28, 2017 15:00
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@gronke gronke mentioned this pull request Feb 28, 2017
@gronke
Copy link
Contributor Author

gronke commented Feb 28, 2017

Since the base branch https://github.com/Polymer/polymer/tree/refactor-import-href was not rebased for a while, the new PR #4348 is pointing against master directly. Closing.

@gronke gronke closed this Feb 28, 2017
@gronke gronke deleted the enhance-refactor-import-href branch August 8, 2017 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants