getsentry / sentry-docs

Sentry's documentation (and tools to build it)
https://docs.sentry.io
Other
332 stars 1.45k forks source link

surprising behavior of `supported`/`notSupported` when platform has fallback #4865

Closed lobsterkatie closed 1 month ago

lobsterkatie commented 2 years ago

Steps to Reproduce

  1. Add an option to the main config/options page
  2. Set the supported flag to ['javascript']
  3. Build the docs
  4. Go to the RN version of the options page

Expected Result

To not see the option, since RN isn't in the list of supported platforms.

Actual Result

I do see the option.

Additional Info

The reason this is happening is the logic used to decide whether or not to show a PlatformSection element (ConfigKey is just a wrapper around PlatformSection).

https://github.com/getsentry/sentry-docs/blob/ccb0eaa8bc7070b25916c568475b9a487e07de9e/src/components/platformSection.tsx#L29-L53

The isSupported() function correctly says that RN doesn't support the option. But because of getPlatformsWithFallback(), javascript is also checked, and because it does have the option, that option ends up showing up on the RN page.

There are lots of circumstances where this is exactly the correct behavior, so we may end up closing this with an "As Designed" label, but it's nonetheless surprising. Guides falling back to their platform is very intuitive. One platform falling back to another is much less so. I don't totally have an answer here, but wanted to raise it as a point of discussion for if and when we ever get a docs engineer.

imatwawana commented 2 years ago

Do you know if the workaround here is that we'd have to use the notSupported flag as well? This behaviour doesn't make a lot of sense to me either.

lobsterkatie commented 2 years ago

Do you know if the workaround here is that we'd have to use the notSupported flag as well?

Yup, that's exactly what we have to do, almost like RN were actually javascript.react-native.

mydea commented 1 month ago

@chargome do you know if this issue is still relevant or possibly already fixed/no longer relevant?

chargome commented 1 month ago

@mydea I don't think this can be issue anymore since the RN platform is completely seperated from js – couldn't reproduce it anymore!