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

feat: migrate to preact/compat #190

Merged
merged 22 commits into from
Apr 16, 2024
Merged

feat: migrate to preact/compat #190

merged 22 commits into from
Apr 16, 2024

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Apr 10, 2024 •
edited
Loading

Title

feat: migrate to preact/compat

Description

Built on top of #188 intended to test migration to preact. The size reduction is 0.5M

Notes & open questions

Things work fine, but preact/compat says suspense and lazy are experimental and shouldn't be used in prod.

Size and assets on fix/dynamic-imported-ui-components

total 4.7M
drwxr-xr-x 33 sgtpooki 1.1K Apr 10 13:41 ..
-rw-r--r--  1 sgtpooki  30K Apr 10 13:41 ipfs-sw-778.js
-rw-r--r--  1 sgtpooki 5.2K Apr 10 13:41 ipfs-sw-778.css
-rw-r--r--  1 sgtpooki  51K Apr 10 13:41 ipfs-sw-main.js
-rw-r--r--  1 sgtpooki  16K Apr 10 13:41 ipfs-sw-781.js
-rw-r--r--  1 sgtpooki  25K Apr 10 13:41 ipfs-sw-850.js
-rw-r--r--  1 sgtpooki 2.5K Apr 10 13:41 ipfs-sw-992.js
-rw-r--r--  1 sgtpooki 2.5K Apr 10 13:41 ipfs-sw-611.js
-rw-r--r--  1 sgtpooki  55K Apr 10 13:41 ipfs-sw-sw.js
-rw-r--r--  1 sgtpooki 2.7K Apr 10 13:41 ipfs-sw-styles.css
-rw-r--r--  1 sgtpooki  57K Apr 10 13:41 ipfs-sw-117.js
-rw-r--r--  1 sgtpooki  223 Apr 10 13:41 ipfs-sw-styles.js
-rw-r--r--  1 sgtpooki  88K Apr 10 13:41 ipfs-sw-96.js
-rw-r--r--  1 sgtpooki   26 Apr 10 13:41 _redirects
-rw-r--r--  1 sgtpooki 577K Apr 10 13:41 ipfs-sw-vendor-react.js
-rw-r--r--  1 sgtpooki 3.4M Apr 10 13:41 ipfs-sw-561.js
-rw-r--r--  1 sgtpooki 6.1K Apr 10 13:41 ipfs-sw-117.js.LICENSE.txt
drwxr-xr-x 31 sgtpooki  992 Apr 10 13:41 .
-rw-r--r--  1 sgtpooki 284K Apr 10 13:41 ipfs-sw-561.js.LICENSE.txt
-rw-r--r--  1 sgtpooki   43 Apr 10 13:41 ipfs-sw-611.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 2.8K Apr 10 13:41 ipfs-sw-781.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 1.9K Apr 10 13:41 ipfs-sw-778.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 8.3K Apr 10 13:41 ipfs-sw-96.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 1.5K Apr 10 13:41 ipfs-sw-850.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 8.3K Apr 10 13:41 ipfs-sw-sw.js.LICENSE.txt
-rw-r--r--  1 sgtpooki   43 Apr 10 13:41 ipfs-sw-992.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 5.9K Apr 10 13:41 ipfs-sw-main.js.LICENSE.txt
-rw-r--r--  1 sgtpooki    6 Apr 10 13:41 ipfs-sw-styles.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 1.7K Apr 10 13:41 index.html
-rw-r--r--  1 sgtpooki  776 Apr 10 13:41 ipfs-sw-vendor-react.js.LICENSE.txt
-rw-r--r--  1 sgtpooki  34K Apr 10 13:41 favicon.ico

Size and assets on this branch

total 4.2M
drwxr-xr-x 33 sgtpooki 1.1K Apr 10 13:37 ..
-rw-r--r--  1 sgtpooki  51K Apr 10 13:37 ipfs-sw-main.js
-rw-r--r--  1 sgtpooki  30K Apr 10 13:37 ipfs-sw-778.js
-rw-r--r--  1 sgtpooki 5.2K Apr 10 13:37 ipfs-sw-778.css
-rw-r--r--  1 sgtpooki  16K Apr 10 13:37 ipfs-sw-781.js
-rw-r--r--  1 sgtpooki  25K Apr 10 13:37 ipfs-sw-850.js
-rw-r--r--  1 sgtpooki 2.5K Apr 10 13:37 ipfs-sw-611.js
-rw-r--r--  1 sgtpooki  55K Apr 10 13:37 ipfs-sw-sw.js
-rw-r--r--  1 sgtpooki 2.5K Apr 10 13:37 ipfs-sw-992.js
-rw-r--r--  1 sgtpooki 2.7K Apr 10 13:37 ipfs-sw-styles.css
-rw-r--r--  1 sgtpooki  223 Apr 10 13:37 ipfs-sw-styles.js
-rw-r--r--  1 sgtpooki  57K Apr 10 13:37 ipfs-sw-117.js
-rw-r--r--  1 sgtpooki   26 Apr 10 13:37 _redirects
-rw-r--r--  1 sgtpooki 174K Apr 10 13:37 ipfs-sw-304.js
-rw-r--r--  1 sgtpooki 6.1K Apr 10 13:37 ipfs-sw-117.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 3.4M Apr 10 13:37 ipfs-sw-561.js
drwxr-xr-x 29 sgtpooki  928 Apr 10 13:37 .
-rw-r--r--  1 sgtpooki   43 Apr 10 13:37 ipfs-sw-611.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 2.1K Apr 10 13:37 ipfs-sw-778.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 8.5K Apr 10 13:37 ipfs-sw-304.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 284K Apr 10 13:37 ipfs-sw-561.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 1.7K Apr 10 13:37 ipfs-sw-850.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 2.9K Apr 10 13:37 ipfs-sw-781.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 6.3K Apr 10 13:37 ipfs-sw-main.js.LICENSE.txt
-rw-r--r--  1 sgtpooki    6 Apr 10 13:37 ipfs-sw-styles.js.LICENSE.txt
-rw-r--r--  1 sgtpooki  34K Apr 10 13:37 favicon.ico
-rw-r--r--  1 sgtpooki   43 Apr 10 13:37 ipfs-sw-992.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 8.3K Apr 10 13:37 ipfs-sw-sw.js.LICENSE.txt
-rw-r--r--  1 sgtpooki 1.6K Apr 10 13:37 index.html

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@SgtPooki SgtPooki requested a review from 2color April 10, 2024 21:16
@SgtPooki SgtPooki self-assigned this Apr 10, 2024
@SgtPooki SgtPooki requested a review from lidel April 10, 2024 21:16
@SgtPooki SgtPooki marked this pull request as ready for review April 10, 2024 21:16
@SgtPooki SgtPooki requested a review from a team April 12, 2024 18:50
@SgtPooki
Copy link
Member Author

FYI: I think the lightweight usage of suspense here should be fine for now.. we can address later if we're really concerned about Suspense/lazy being experimental. lmk if you want me to fix now, and I can do some async finagling.

Base automatically changed from fix/dynamic-imported-ui-components to main April 12, 2024 23:46
lidel
lidel reviewed Apr 15, 2024
View reviewed changes
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

We are down to ~4.2MiB uncompressed, really nice!

I've built with npm ci && npm run build and tested with cd dist && python -m http.server 3000, found small cosmetic problem below, other than that, lgtm.

src/pages/config.tsx
Copy link
Member

Choose a reason for hiding this comment

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

Seems there is a small cosmetic(?) regression when I open http://sw.localhost:{port}/#/ipfs-sw-config and try to customize gateways and then save them.

Pressing on Save Config button seems to save the new value, but it does not switch me back to http://sw.localhost:{port}/, like it did in the react version, I am stuck on the config screen:

2024-04-15-235433_1027x585_scrot

Copy link
Member Author

Choose a reason for hiding this comment

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

This changed slightly recently. Ill double check

Copy link
Member Author

Choose a reason for hiding this comment

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

So weird. I was not able to reproduce this, then I was, then I wasn't... I will merge main and then look some more.

Copy link
Member Author

Choose a reason for hiding this comment

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

2024-04-16.at.12.07.18.-.Emerald.Harrier.mp4

Copy link
Member Author

Choose a reason for hiding this comment

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

you know what it was, my dev server was shut down and the redirect didn't work when it was down. it works when it stays up..

@SgtPooki
Copy link
Member Author

config page save -> redirect is happening with npx http-server dist as well. I think it's a non-issue? we can address later if someone can replicate that problem, I can't right now!

@SgtPooki SgtPooki merged commit 8c98f57 into main Apr 16, 2024
20 checks passed
@SgtPooki SgtPooki deleted the feat/preact-migration branch April 16, 2024 19:53
@lidel lidel mentioned this pull request Apr 16, 2024
Merged
SgtPooki added a commit that referenced this pull request Apr 24, 2024
@SgtPooki
Revert "feat: migrate to preact/compat (#190)"
24f5d3d 
This reverts commit 8c98f57.
@SgtPooki SgtPooki mentioned this pull request Apr 24, 2024
Merged
3 tasks
SgtPooki added a commit that referenced this pull request Apr 24, 2024
@SgtPooki
fix: revert "feat: migrate to preact/compat (#190)" (#219)
4443174 
This reverts commit 8c98f57.
@lidel lidel mentioned this pull request Apr 24, 2024
Merged
SgtPooki added a commit that referenced this pull request Apr 24, 2024
@SgtPooki
fix: multiple bugs (#220)
75aa0b8 
* Revert "feat: migrate to preact/compat (#190)"

This reverts commit 8c98f57.

* fix: duplicate body processing in errorPageResponse

* fix: match css + js sw assets

* fix: sw sometimes uses empty gateways+routers config

* fix: getConfig is more robust and receives logger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel left review comments

@2color 2color Awaiting requested review from 2color

Assignees

@SgtPooki SgtPooki

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SgtPooki @lidel