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

add pwa debug toolbar #12

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

tacman
Copy link
Contributor

@tacman tacman commented Mar 9, 2024

my branch ('tac') diverged enough from the demo that I created a new branch from the most recent version of 'main', with just pwa-extra-bundle added.

It adds the debug toolbar. Still needs some work, but it's been helpful to me.

To see the strategies in action, turn on Preserve Log in the Chrome Console tab. The go to one of the url tabs, when you load a URL you can see it in action. Depending on the strategy, you should also be able to do all that offline.

image

@Spomky
Copy link
Member

Spomky commented Mar 9, 2024

Impressive!
Tell me when it is OK for me and I will merge this PR.

@Spomky
Copy link
Member

Spomky commented Mar 9, 2024

Do you think the maxAge could be converted into a human readable string?

@tacman
Copy link
Contributor Author

tacman commented Mar 9, 2024

maxAge could be converted into a human readable string?

yeah, I'll do that. What else are we missing?

I think having the toolbar reflect the cacheStrategy would be kinda cool. Since we now have the warmCache urls, we can easily check for those at least.

I would like to see what this looks like with more caching strategies, especially the pages. I don'tthink the syntax is right yet, but I think an Attribute is the way to go.

How about I keep working on the debug toolbar a bit (I'm learning a bunch about the system by doing this!). Something doesn't feel right when offline, I'll post that as another issue in a bit. But I'd love to have a way to define page cache strategies, in the YAML as well as in an eventsubscriber and a route attribute. The debug toolbar will undoubtedly be helpful for making sure it's working as expected.

@Spomky
Copy link
Member

Spomky commented Mar 9, 2024

I would like to see what this looks like with more caching strategies, especially the pages. I don'tthink the syntax is right yet, but I think an Attribute is the way to go.

your wish is granted. See Spomky-Labs/pwa-bundle#123
Not finished. At the moment it supports only SWR and NF strategies

EDIT: Does not use attributes, but that's one of the first steps

@Spomky
Copy link
Member

Spomky commented Mar 9, 2024

The debug toolbar is very very nice.

One remark though: a PWA may have a Manifest and/or a SW
To me, the toolbar should have 2 tabs: Manifest and Service Worker.
In the SW tab, we will then list all caching strategies, but also other feature statuses (skip waiting, scope...).
WDYT?

@tacman
Copy link
Contributor Author

tacman commented Mar 9, 2024

Yeah, we could certainly separate them, the manifest could then have separate sections for the icons and screenshots.

Regarding the service worker, I thought it'd be awesome to have information about the actual service worker (not just the configuration), so I asked if it was even possible.

Having two separate toolbars (manifest and service worker) means 2 datacollectors, which I can do at some point.

Speaking of the manifest, can you provide a url that I can use to display the icon? This hack isn't working

                <h3>{{ manifest.icons|length }} icons</h3>
                <p>@todo: map from /assets to a link we use use in an img tag</p>
                {% for icon in manifest.icons %}
                    <li>

                        {% set imageUrl = icon.src|replace({'/assets': ''}) %}
                        <a href="{{ imageUrl }}">
                            <img style="max-width: 100px" src="{{ imageUrl }}" title="{{ icon.src }}"
                                 alt="{{ imageUrl }}">
                        </a>
                        <br/>Sizes: {{ icon.sizes|join(',') }}
                    </li>
                {% endfor %}

@tacman
Copy link
Contributor Author

tacman commented Mar 10, 2024

I think Background Sync is going to need it's own tab. Once I get the demo working, I'll incorporate some ideas.

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.

2 participants