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.

Sign up for GitHub

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

Jump to bottom

chore: bump pub-sub-es to latest #1095

Merged
merged 1 commit into from
Apr 16, 2022
Merged

chore: bump pub-sub-es to latest #1095

merged 1 commit into from
Apr 16, 2022

Conversation

manzt
Copy link
Member

@manzt manzt commented Apr 15, 2022 •
edited

Description

Why is it necessary?

I've been working on several applications with both HiGlass/Gosling where I'm generating HTML on the fly for the web-browser. I'd like to avoid writing the HTML to disk, and instead load the content via a data url (e.g. data:text/html,<doctype!><html><h1>Hey</h1></html>). Currently this throws an unrecoverable error due to an edge case with the version of pub-sub-es that higlass is on. This can additionally be useful with puppeteer for taking screenshots:

import puppeteer from "puppeteer";

let viewconf = JSON.stringify(...);
let html = `\
<!doctype html>
<head>
  <link rel="stylesheet" href="https://unpkg.com/higlass@1.7/dist/hglib.css">
  <script src="https://unpkg.com/react@17/umd/react.production.min.js"></script>
  <script src="https://unpkg.com/react-dom@17/umd/react-dom.production.min.js"></script>
  <script src="https://unpkg.com/pixi.js@6/dist/browser/pixi.min.js"></script>
  <script src="https://unpkg.com/higlass@1.7/dist/hglib.min.js"></script>
</head>
<body></body>
<script>
	hglib.viewer(document.body, JSON.parse(\`${viewconf}\`);
</script>
</html>`;

let browser = await puppeteer.launch();
let page = await browser.newPage();
await page.setContent(html, { waitUntil: "networkidle0" });
let component = await page.waitForSelector(".higlass");
await component.screenshot({ path: "./example.jpeg" });
await browser.close();

Details

pub-sub-es instantiates a browser BroadcastChannel if available on import to broadcast events from the gloablPubSub. Instantiating a BroadcastChannel throws if the website does not contain an origin, which is the case when the page content is the url. The previous version of pub-sub-es doesn't catch this edge case,

pub-sub-es v1.2

const bc = (() => {
  const BC = window.BroadcastChannel;
  if (!BC) {
    console.warn("The Broadcast Channel API is not available in your browser.");
    return { postMessage: () => {} };
  }
  return new BC("pub-sub-es"); // throws because no origin, not caught
})();

while the latest version wraps the instantiation in a blanket try/catch.

pub-sub-es v2

const bc = (() => {
  try {
    return new window.BroadcastChannel('pub-sub-es');
  } catch (e) {
    return { postMessage: () => {} };
  }
})();

Breaking changes?

From what I can tell, the breaking changes in v2 are changes to options parameter for createPubSub, which higlass does not rely on. @flekschas

https://github.com/flekschas/pub-sub/blob/master/CHANGELOG.md

Checklist

  • Set proper GitHub labels (e.g. v1.6+, ignore if you don't know)
  • Unit tests added or updated
  • Documentation added or updated
  • Example(s) added or updated
  • Update schema.json if there are changes to the viewconf JSON structure format
  • Screenshot for visual changes (e.g. new tracks or UI changes)
  • Updated CHANGELOG.md

flekschas
flekschas approved these changes Apr 16, 2022
View reviewed changes
Copy link
Member

@flekschas flekschas left a comment

Choose a reason for hiding this comment

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

Should be a safe update 👍

@flekschas flekschas merged commit efbe9d9 into develop Apr 16, 2022
@flekschas flekschas deleted the manzt/bump-pubsub-es branch April 16, 2022 17:32
@manzt
Copy link
Member Author

manzt commented Apr 19, 2022

Any chance we could cut a release? @pkerpedjiev

@manzt manzt mentioned this pull request Apr 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flekschas flekschas flekschas approved these changes

@pkerpedjiev pkerpedjiev Awaiting requested review from pkerpedjiev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@manzt @flekschas