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

Event model is flaky #26

Open
arthurevans opened this issue Oct 5, 2016 · 2 comments
Open

Event model is flaky #26

arthurevans opened this issue Oct 5, 2016 · 2 comments

Comments

@arthurevans
Copy link

This is kind of a meta-bug, related to issues reported on the catalog (googlearchive/polymer-element-catalog#207) and elsewhere.

Briefly, the ordering of attached can vary based on whether you're polyfilled or not, so the current interaction model for this element is racy.

I think this was an early experiment in loosely-coupled components, but the protocol isn't reliable.

I suggest that we could either make it more reliable, or simply turn prism-element into a prism-behavior that exposes a single highlight(code, language-hint) function. Which would remove the loose coupling benefits, but be fast and reliable.

... Or we tell users they should fire the event, and see if they get results, and if not, wait a rAF and try again.

runn-vermel added a commit to runn-vermel/prism-element that referenced this issue Oct 13, 2016
Firing event when highlight event listener is registered

If you place prism highlighter in another component, there's no way to know when the highlight event listener is registered, resulting in a possible race condition - highlight events being sent out before the event listener is even registered.

By firing a (prism-highlighter-highlight-event-registered) event after the listener is registered, the host can listen for the fired event, and act accordingly.

I've also improved shadow dom documentation, and added references to how to make shadow dom work.
@FredKSchott
Copy link
Contributor

@arthurevans thanks for investigating & writing this up! That's a bummer that the protocol isn't more reliable in the polyfill. This is the first I've heard of this, but I'm assuming it's non-trivial or impossible to make the polyfill match the spec here? If not, I think it makes sense to build a prism-behavior, and then move this element into maintenance mode with a warning about the race condition and a link to the new behavior.

@notwaldorf is the real brains behind this element though, I'm just the janitor :) wdyt?

@arthurevans
Copy link
Author

I don't think we can fix the attached timing in the polyfill (@sorvell or @azakus can probably confirm there). And I suspect that we don't want to encourage this pattern (adding an event listener on the parent), which is not in the spirit of the mediator pattern.

3 solutions, from simplest to most complex:

  1. Just import the library (prism-import.html) and use the global. (Downside: might need to copy the tiny bit of language-guessing logic.)

  2. Add a behavior that just exposes highlight (including the language-guessing logic).

  3. Add behavior as above, but also provide an _addHighlightListener method, so a parent element could provide a drop-in replacement without updating its children, and the <prism-element> could be trivially reimplemented using the behavior.

In addition to the reliability issues, I suspect firing one event per highlight request is less performant than a simple method call, especially for a page with a lot of small code blocks.

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

No branches or pull requests

2 participants