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

Modify fyne package -os web to generate a PWA with empty service worker, update package unit tests #5098

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

joebologna
Copy link

@joebologna joebologna commented Aug 27, 2024

Description:

Using fyne package -os web creates an index.html that cannot be installed as a PWA.

Modified the packager to include a manifest.json, service-worker.js and update index.html to run the service-worker.js. This allows installing the PWA using the browser. The resulting PWA is a binary for the target platform that is generated by the browser itself. Of course, it still runs a captive browser in the application.

The proposed fix has an empty service-worker.js, which allows installing the PWA but the functions normally performed by the service worker, such as caching data and support for updates is not included in this enhancement. Adding a functional service worker requires consideration for what makes sense to support when running a WASM payload in a browser. This enhancement makes it possible for someone in the future to craft functional service worker based on features it should support. It will likely need someone expert in the construction of service workers for browser to design and implement.

Fixes #5097

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

Where applicable:

  • Public APIs match existing style and have Since: line.
  • Any breaking changes have a deprecation path or have been discussed.
  • Check for binary size increases when importing new modules.

@coveralls
Copy link

Coverage Status

coverage: 66.042% (-0.006%) from 66.048%
when pulling b6abbcf on joebologna:develop
into de71490 on fyne-io:develop.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks for adding this.

Are you sure a no-op worker is required to make this installable? Wont the metadata be sufficient?

@joebologna
Copy link
Author

joebologna commented Aug 28, 2024

You are correct, a service worker is not required. The quality of a PWA is measured by its Lighthouse score. The score is generated using the Developer Tools in Chrome.

Not having a service worker will reduce the score by not providing optimization. Since the webpage is just running a WASM payload there isn't a need for a service worker to cache other data. A functional service worker and/or more JavaScript may be required to indicate an update is available and allow it to be refreshed in the PWA. Without the refresh capability may be difficult to get the new WASM payload into the app.

In summary, without doing some research on the refresh capability, the value installing the app as a PWA is dubious.

In my opinion, these are the options:

  1. Abandon the enhancement until more research can be done
  2. Determine what PWA features are desired/required, refreshing the PWA might be the only one to consider
  3. Leave the code to generate service-worker.js and run it but include comments in the .js file to allow modifying itto add the functionality after the web app is built - this kludgy. It would probably be best to allow customization akin to the method used with FyneApp.toml or AndroidManifest.xml

I'm partial to option 2.

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

Successfully merging this pull request may close these issues.

3 participants