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

disable frontend version check #6194

Closed
alexan opened this issue Mar 20, 2019 · 34 comments
Closed

disable frontend version check #6194

alexan opened this issue Mar 20, 2019 · 34 comments
Assignees
Milestone

Comments

@alexan
Copy link
Contributor

alexan commented Mar 20, 2019

Is your feature request related to a problem? Please describe.
On our infrastructure we don't allow requests to other domains for security reasons.
Since 5.x of storybook the frontend does requests to https://storybook.js.org/versions.json to notify the user of new releases. This produces error logs in the javascript console.

Describe the solution you'd like
I would like to have a cli switch and/or config parameter to disable this version check.

Describe alternatives you've considered
Maybe is is possible to detect the content security settings in js and don't do the calls if forbidden.
I think this won't be as straight forward and disabling this check could have various other reasons.

Are you able to assist bring the feature to reality?
yes, I can...

Additional context
Bildschirmfoto 2019-03-20 um 13 18 34

@shilman
Copy link
Member

shilman commented Mar 22, 2019

Hi @alexan, in general we'd like people to run the update check to keep their storybook up to date, and we may also use that for distributing security notices in the future. However in cases like yours it seems reasonable to disable it. Do you think you might be able to add the CLI flag --disable-update-check to turn off the version update?

@shilman shilman added this to the v5.1.0 milestone Mar 22, 2019
@backbone87
Copy link
Contributor

imho the version check should be removed from the static build. it does not look very nice, if you present it to your companies other departments.

@ghengeveld
Copy link
Member

ghengeveld commented Mar 25, 2019

+1 on removing it from the static build. Makes no sense to show the "upgrade available" notification badge in a production environment.

I also suggest to show the notification only for minor/major releases. With patch releases several times a week it's not feasible to stay up-to-date all the time. Relying on version ranges (^x.x.x) is also not an option, as we don't allow automatic updates, even for patch releases.

@tomspeak
Copy link

Would also appreciate this being an optional thing, or possible to have it only in a local dev environment.

@stale
Copy link

stale bot commented Apr 16, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Apr 16, 2019
@Haroenv
Copy link
Contributor

Haroenv commented Apr 16, 2019

I'm also wondering if every patch version needs to be promoted so heavily in the build, especially in a case like this, where 5.0.7 & 5.0.8 are exactly the same

@stale stale bot removed the inactive label Apr 16, 2019
@shilman
Copy link
Member

shilman commented Apr 16, 2019

@Haroenv Currently the only thing that should show up in a patch version is a little green circle on the menu. Are you seeing something else?

@Haroenv
Copy link
Contributor

Haroenv commented Apr 16, 2019

I just realised I didn’t comment on the right issue, but my terminal has “you should update to 5.0.8”, when on 5.0.7

@shilman
Copy link
Member

shilman commented Apr 16, 2019

@Haroenv I see -- yeah that's something I could get behind. Will discuss with the team and see what we can do!

@tofran
Copy link

tofran commented Apr 17, 2019

While this is not implemented I've made a simple workaround (for the backed side only):

https://gist.github.com/tofran/949060c7e385c66bfb76efa043479028

@stale
Copy link

stale bot commented May 8, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label May 8, 2019
@tofran
Copy link

tofran commented May 28, 2019

I think this is very relevant and should not be closed.
This feature may be used for telemetry purposes. Users should be able to opt-ot from it.

@stale stale bot removed the inactive label May 28, 2019
@chrismiceli
Copy link

I agree that this should be resolved. I would even further argue off by default, at least until I can find a privacy policy for the data collected from these update checks. This may be a legal issue in some countries.

@shilman shilman modified the milestones: 5.1.0, 5.1.x Jun 5, 2019
@stale
Copy link

stale bot commented Jun 26, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jun 26, 2019
@chrismiceli
Copy link

Can we get a comment from project maintainers that a PR would even be accepted for this feature before someone invest time into resolving it?

@stale stale bot removed the inactive label Jun 27, 2019
@stale
Copy link

stale bot commented Jul 18, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@chrismiceli
Copy link

Ping asking for comment on a whether a PR would be approved or not.

@stale stale bot removed the inactive label Jul 19, 2019
@thibaudcolas
Copy link

thibaudcolas commented Aug 8, 2019

I would also like to have a way to turn this off. The main reason is that I find the current design of notifications quite obnoxious. Then, also because I wouldn’t use this type of in-app information to keep track of new versions to start with. I also wouldn’t use this for security notices as @shilman suggests – I already use other channels for this as well, that are not specific to Storybook.

Back to the design,

notifications

Here is what I don’t like about it:

  • It often sits above the main Storybook navigation
  • Its "light on dark" theme makes it visually distracting as it stands out a lot compared to the rest of the Storybook UI, and my stories.
  • There is no clear indication that it’s clickable (it does not have any of the visual cues of links or buttons), so it took me at least a couple of months of seeing the notification literally all the time, to realise this was something I could click on to make it disappear. I wouldn’t be surprised if there were lots of people in the same situation.
  • There is no way to dismiss it without opening the "About" page. Notifications should be dismiss-able without having to trigger what the notification is for to start with.
  • The "🎉" and the exclamation mark make it look like this information is something to rejoice about. I think I would appreciate like this kind of tone if I was wanting to keep up with Storybook updates (either opting in to a notification like this, or because I browsed recent updates on the project’s website). But here, while I’m working on my project, this feels more like an ad than anything.

There is also the concerns of privacy that @tofran and @chrismiceli have raised. Generally I would expect any kind of "phone home" feature in open-source software to either be disable-able, or opt-in only, regardless of whether it’s actually used for telemetry or not.

From a legal perspective, I think the first obvious source of concern in the EU would be cookie laws – the version check is storing information in the user’s browser, without their consent, and I don’t think this information can be construed to be essential to Storybook being functional. I’m not entirely sure whether this would definitely be illegal or not, but I can see why people would want to disable this check for that reason alone. See https://ico.org.uk/about-the-ico/news-and-events/news-and-blogs/2019/07/blog-cookies-what-does-good-look-like/ if you’re interested in this.

Anyway, I’d be happy to make a PR for this if we can get some guidance from the Storybook team on how they would like to see this implemented – both disabling the version check completely, and improving the design of the notifications if there is some agreement on my concerns above (for example adding a "close" icon on the right-hand side).

@stale
Copy link

stale bot commented Aug 29, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Aug 29, 2019
@Kikobeats
Copy link

keep this open, my friendly bot

@stale stale bot removed the inactive label Aug 29, 2019
@dan-diaz
Copy link

my team is also not very happy with this "update floaty"

@thibaudcolas
Copy link

I think the only reason why this is inactive is that we’re waiting for some guidance from the Storybook team that they’re happy to take PRs to disable this in-browser version check, whether that’s via a CLI flag, environment variable, or some other form of configuration.

@shilman you were mentioning discussing this with the wider team a few months ago (#6194 (comment)), is there an update on this?

@shilman
Copy link
Member

shilman commented Aug 29, 2019

Yeah we're open to backing off on this. We're in the final stages of getting 5.2 out the door and will prioritize a fix first thing in 5.3.

@shilman shilman modified the milestones: 5.1.x, 5.3.0 Aug 29, 2019
@wbern
Copy link

wbern commented Sep 18, 2019

In the mean time, here's a workaround to hide it.

  1. Create manager-head.html in .storybook directory.
  2. Put in following contents:
<style type="text/css">
    [href='/?path=/settings/about'] {
        display: none;
    }
</style>

@chrismiceli
Copy link

My bigger concern is the phone home telemetry issues, though the popup is annoying as well.

@brenzy
Copy link

brenzy commented Sep 22, 2019

For the workaround above I needed to add !important for it to work:

<style type="text/css">
    [href='/?path=/settings/about'] {
        display: none !important;
    }
</style>

@raphaellueckl
Copy link

How come this issue does not get any attention? Why should our users be informed, that there is a new storybook version, if they don't even have control over the codebase? That we get those messages during development is nice, but that we have to find workarounds to suppress those messages on PROD is kind of weird. Our users often don't even know what "Storybook" is, so this can get even more confusing.

@thibaudcolas
Copy link

thibaudcolas commented Sep 24, 2019

@raphaellueckl as far as I can see the notifications have been disabled for production builds since v5.0.3 / 5.1.0-alpha.9: UI: Make update notifications much less aggressive (#6143). If you’re running a more recent version than this and still have the notifications that’s likely a bug / unintended.

@raphaellueckl
Copy link

raphaellueckl commented Sep 24, 2019

Hi @thibaudcolas

Thanks for your response! :)

We are using the following versions (currently wondering why the html package is still 5.0.8):

"@storybook/addon-knobs": "^5.1.9",
"@storybook/addons": "^5.1.9",
"@storybook/html": "^5.0.8",
"@storybook/theming": "^5.1.9",

With "production" I mean the statically files coming npm run build-storybook.

@shilman
Copy link
Member

shilman commented Sep 24, 2019

@raphaellueckl coming soon in 5.3. we've been busy shipping 5.2.

@raphaellueckl
Copy link

@shilman Cool, thank you guys!

@shilman
Copy link
Member

shilman commented Oct 24, 2019

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.27 containing PR #8488 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Oct 24, 2019
@JackHowa
Copy link

this worked for me with !important, thanks @brenzy and @wbern

        display: none !important;

JackHowa pushed a commit to WPMedia/engine-theme-sdk that referenced this issue Dec 14, 2020
@ferdicus
Copy link

ferdicus commented Feb 9, 2024

i know, this is an old issue, however i'd like to ask:
is this also possible as a config option instead of a cli flag?

thanks in advance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests