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

[gatsby-plugin-offline] Embedding Greenhouse #26058

Closed
ElijahLynn opened this issue Jul 27, 2020 · 9 comments
Closed

[gatsby-plugin-offline] Embedding Greenhouse #26058

ElijahLynn opened this issue Jul 27, 2020 · 9 comments
Labels
stale? Issue that may be closed soon due to the original author not responding any more. topic: plugins-PWA Issues related to PWA: the gatsby-plugin-offline and gatsby-plugin-manifest plugins type: bug An issue or pull request relating to a bug in Gatsby

Comments

@ElijahLynn
Copy link
Contributor

Description

Per https://app3.greenhouse.io/jobboard/integration/documentation/embedded_board_and_apps we embedded a Greenhouse.io script <script src="https://boards.greenhouse.io/embed/job_board/js?for=agilesix"></script> on our site and it loads fine the first time and the script then attaches an iframe to the target <div id="grnhse_app"></div> correctly. A subsequent reload does not execute the script though. It appears the script is only executed on initial load. A "unregister serviceworker" does allow it to be loaded on a normal refresh again.

Steps to reproduce

Clear steps describing how to reproduce the issue. Please please please link to a demo project if possible, this makes your issue much easier to diagnose (seriously).

  1. Add <div id="grnhse_app"></div>
  2. Then below add <script src="https://boards.greenhouse.io/embed/job_board/js?for=agilesix"></script>
  3. Then Load page, works fine.
  4. Second load/or refresh does not work
  5. Unregister the service worker, refresh page, works fine.

How to Make a Minimal Reproduction: https://www.gatsbyjs.org/contributing/how-to-make-a-reproducible-test-case/

Expected result

Iframe should be attached every load

Actual result

Iframe (script execution) is only attached on first load, not on subsequent loads.

Environment

Run gatsby info --clipboard in your project directory and paste the output here.

@ElijahLynn ElijahLynn added the type: bug An issue or pull request relating to a bug in Gatsby label Jul 27, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 27, 2020
@ElijahLynn ElijahLynn added the topic: plugins-PWA Issues related to PWA: the gatsby-plugin-offline and gatsby-plugin-manifest plugins label Jul 27, 2020
@pvdz
Copy link
Contributor

pvdz commented Jul 28, 2020

Hi @ElijahLynn, it would be helpful if you could post a repo with the repro. The person who initially created this plugin no longer works at Gatsby so we don't really have an expert available at the moment. To this end, a repo with repro would be most helpful.

From the top of my head I'd guess that the SW is somehow bypassing the script, considering it's being offline and all, but it's been a while since I worked with that part myself.

@ElijahLynn
Copy link
Contributor Author

Thanks! We just posted a PR with the commit it is at to reproduce as well as a working Netlify environment to test above on.

agilesix/AgileSix.com#34
https://deploy-preview-34--agile6.netlify.app/team/ (scroll to bottom for Greenhouse job board iframe)

@pvdz pvdz removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 29, 2020
@pvdz
Copy link
Contributor

pvdz commented Jul 29, 2020

Would you not just want to remove the offline plugin from the config? Comment out https://github.com/agilesix/AgileSix.com/blob/master/gatsby-config.js#L110 and be done with it.

Yes you lose offline support, but it sounds like the greenhouse integration is more important to you. If the greenhouse integration is supposed to work with service workers then I'd expect to have to be added to the sw manifest etc. Maybe greenhouse offers a solution for this, I don't know.

I'm inclined to mark this "works as intended"...

Anyways, I built the site in that branch. The team page does not seem to load anything related to greenhouse, at all.

Also, sorry about the build time. That's mostly about image scrubbing, although the JS webpack bundle seems big as well (this site creates a lot of JS in bundles, have you looked into that?).

You can probably improve the build time by preprocessing those images and maybe by using the bundle analyzer plugin to figure out what JS is being shipped exactly.

Do you think there's a bug to work out or is the removal of the offline plugin sufficient?

@ElijahLynn
Copy link
Contributor Author

Thanks for the follow up. Removing offline support is possible, but worth noting that the offline support also allows for faster cache retrieval as Service Worker Cache is much faster than browser cache. And browsing sites with SWs is just so snappy and fast I would not want to remove it just yet without trying a bit more.

I think there is a bug here and that anyone who tries to use a script like this will only have it executed the first time and not subsequent times, so it will affect others as well. I suspect there is some browser event hook that doesn't fire when it is loaded from the SW Cache. We should probably rename this ticket to something more generic, like "script tag not executing on refresh".

I can reach out to Greenhouse team and link them to this ticket as well.

Also, sorry about the build time. That's mostly about image scrubbing, although the JS webpack bundle seems big as well (this site creates a lot of JS in bundles, have you looked into that?).

Thanks for going above and beyond here and mentioning this, was wondering why it took so long! We'll look into the Webpack.

@pvdz
Copy link
Contributor

pvdz commented Aug 3, 2020

worth noting that the offline support also allows for faster cache retrieval as Service Worker Cache is much faster than browser cache
I think there is a bug here

While the service worker cache is faster I genuinely wonder whether not loading any remote scripts isn't a feature, rather than a bug. The greenhouse script would need to be part of the manifest in order to be served, and as such, would need to work with a local queue of events, to send remotely once internet connection has been established, etc. At least, that's how I would interpret it.

That said I have no problem if it were possible to get this to work anyways. Just trying to make sure this isn't something that's not possible by design. And beyond the scope of Gatsby, as well :p

Thanks for going above and beyond here and mentioning this

I am the perf engineer after all 😅

@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Aug 24, 2020
@ElijahLynn
Copy link
Contributor Author

Sorry, haven't had time to follow up on this. It is stale,we could close it as long as I can re-open it when I get back to it.

My next course of action will be adding greenhouse.io to the manifest to see how that affects things.

The greenhouse script would need to be part of the manifest in order to be served

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label Aug 26, 2020
@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Sep 15, 2020
@github-actions
Copy link

Hey again!

It’s been 60 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.
Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to comment on this issue or create a new one if you need anything else.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! 💪💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale? Issue that may be closed soon due to the original author not responding any more. topic: plugins-PWA Issues related to PWA: the gatsby-plugin-offline and gatsby-plugin-manifest plugins type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

2 participants