Skip to content

Conversation

@Westbrook
Copy link
Contributor

Description

  • make CEM smaller, about 8.2MB down to 650KB!
  • slim down the images in the Thumbnail stories, about 1.1MB down to 400KB
  • turn the service worker in the Docs Site off at dev time.
  • make CSS watching incremental and properly segment "build CSS" from "build on CSS file"
  • enhance caching for low change files in dev time Storybook

Motivation and context

Found these working through the work in #2257

How has this been tested?

  • boot up the Docs Site locally
    • clear the service work from the Application cache
    • reload the site
  • boot up Storybook
    • see that it doesn't redirect you to the Docs site in the service worker cache
    • see that it's much much faster

Types of changes

  • DX

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions
Copy link
Contributor

github-actions bot commented May 23, 2022

Tachometer results

Chrome

thumbnail permalink

Version Bytes Avg Time vs remote vs branch
npm latest 826 kB 60.26ms - 61.21ms - unsure 🔍
-0% - +1%
-0.19ms - +0.81ms
branch 835 kB 60.26ms - 60.59ms unsure 🔍
-1% - +0%
-0.81ms - +0.19ms
-
Firefox

thumbnail permalink

Version Bytes Avg Time vs remote vs branch
npm latest 826 kB 173.98ms - 181.74ms - unsure 🔍
-2% - +3%
-4.15ms - +5.79ms
branch 835 kB 173.93ms - 180.15ms unsure 🔍
-3% - +2%
-5.79ms - +4.15ms
-

Copy link
Contributor

@hunterloftis hunterloftis left a comment

Choose a reason for hiding this comment

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

Tested via:

yarn
yarn docs:watch
yarn storybook # then changing stuff to see the story update

yarn docs:watch works great, & no longer registers a serviceworker

yarn storybook mostly works, with some small side-effects:

  • forces https (so you get a security warning)... http2?
  • I made some changes to see how quickly storybook updated. It updated very quickly, but I noticed that it updated twice (about 8s apart):

image

@Westbrook
Copy link
Contributor Author

Yes, https comes from the http2 option. It’s likely the least important of these changes, though I do like the update it brings for load speed. I already allow HTTPS for localhost so I didn’t even really notice it. I can look at including a cert in repo, if not do you see that as a blocker?

for the double reload, do you remember what file you updated? Would love to track that down if we can. There is likely still the issue that as best as I can see there’s not way to control what file changes the page reload is bound to, so if you change a CSS file, it will start to reload and if the associated TS file hasn’t been created to subsequently create a JS file and that file isn’t available until that first load the creation of those file trigger subsequent reloads. A PR into web dev server might be in order…

@hunterloftis
Copy link
Contributor

I don't see https as a blocker, just wanted to mention the difference - maybe we could link to some documentation on allowing local https without warnings.

I was updating buttonContent in ButtonBase.ts with:

<div id="label">
  Updated
</div>

@hunterloftis
Copy link
Contributor

(pulled & tested the latest version w/ the same result btw - whether or not that's a blocker I think depends on if multiple-reloads is working as intended)

@Westbrook
Copy link
Contributor Author

Sorry, I updated the branch, but not actually anything in this PR, yet.

I've been testing and it looks like the "incremental build" in tsc isnt' really incremental (or maybe just less incremental than I expected) and touches a large number of files in the build scope no matter what. This means that there is initial refresh and subsequent refresh work no matter what. 😞 Sorry, I didn't test this more outside of the esbuild environment where this first was added to be able to clarify the workflow here for the PR description.

With that in mind, I've just pushed a new commit that filters to a smaller number of files the watch command for Storybook, so that it should refresh a little less often, but with the realities of tsc, I'm not sure we benefit from investing too much more deeply into this before the move to esbuild in the DEV MODE work... there we can decide whether the tighter incremental build is actually working for us or if we need to move to a more custom watch process for WDS in these contexts.

Copy link
Contributor

@hunterloftis hunterloftis left a comment

Choose a reason for hiding this comment

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

Everything works! We can look into reducing refresh counts over time.

@Westbrook Westbrook merged commit 736c049 into main May 25, 2022
@Westbrook Westbrook deleted the dev-perf branch May 25, 2022 20:37
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