-
Notifications
You must be signed in to change notification settings - Fork 24
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
Show wk user in vx workflow list and enable cache busting for bundle chunks #7854
Conversation
877eafa
to
553cc4f
Compare
74083e9
to
6acc3b7
Compare
"string-width-cjs@npm:string-width@^4.2.0": | ||
version "4.2.3" | ||
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" | ||
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== | ||
dependencies: | ||
emoji-regex "^8.0.0" | ||
is-fullwidth-code-point "^3.0.0" | ||
strip-ansi "^6.0.1" | ||
|
||
string-width@^4.0.0, string-width@^4.2.2, string-width@^4.2.3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we think this might be due to changes that happened during yarn add
/ yarn install
and that this shouldnt be a problem.
@@ -84,9 +89,18 @@ export default function WorkflowListView() { | |||
} | |||
} | |||
|
|||
usePolling(loadData, VX_POLLING_INTERVAL); | |||
usePolling(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fm3 I tried to address your comment about requesting the workflows twice while trying not to mess around in the code too much. I thought I could delete the loadData
call where the search query is persisted, but then a dataless page is rendered. So I think this check here might be an acceptable solution for now, especially because the interval is set to null, but I am keen to hear what you say :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
webpack changes look good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for investigating the cache problems! Weird of the browsers that ctrl+shift+r cache clear apparently did not work for the sub-bundles…
I created a follow-up issue to properly clean up the pollingcode #7862
I’ll add a changelog entry to mention the cache fix. Then this should be good to go :)
New attempt to merge old PR #7794 after deployment didnt show the right changes. We found out that the JS bundles are chunked, and the chunk that holds the changes of this PR did not include any cache busting mechanism.
Changes beside old PR:
URL of deployed dev instance (used for testing):
Steps to test:
ToDos
Issues:
(Please delete unneeded items, merge only when none are left open)