Change the fragmentId to be a required property in WidgetStateManager

[LukasMasuch]

Describe your changes

The fragmentId is currently an optional property for all the set state methods in the WidgetStateManager. The reason here is that it sometimes has a value and sometimes is just undefined. But this makes it a bit too easy to forget to add it -> which can lead to features that are not compatible with fragments.

This PR changes the property to required by changing from fragmentId?: string to fragmentId: string | undefined. This also uncovered a potentially existing issue with form submission & fragments, which will be fixed via this PR, e.g.:

https://github.com/streamlit/streamlit/pull/8533/files#diff-b6b2b8d7411dc40d04d4dc0442572c4835e84a79844cd2e299a469b7b2ec1a1cR309

Testing Plan

• Update tests

---

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

[vdonato]

Hm, so I think it'd probably be a good idea to convert this PR into a draft for now and pick up the work again when removing the experimental_ prefix from the fragment decorator. We'll need to do some coordination work with other teams to ensure that this doesn't end up resulting in any backwards compatibility issues, and it'll be easy to focus on that when working on other fragment-relate tasks.

Happy to take this PR over once I start working on fragment de-experimentalization in about a month or so.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sfc-gh-vdonato]

Just did a quick read-through of the notebooks code importing things from @streamlit/lib, and there doesn't seem to be any usage of any of the methods that we're changing the signatures of here.

Probably could have done this far earlier (like while initially implementing fragments so that this refactor wouldn't have been required to begin with), whoops 🙈

[LukasMasuch]

LGTM 👍

There are also two other refactorings related to this I was thinking about that might be worth to consider / discuss as some follow ups PR:

1. Move the formId into its own parmater similar to fragmentId. This is a bit of a problem with effect dependencies that we currently pass the element object as first parameter -> and the element object doesn't work well as effect dependencies since it doesn't check for deep equality only object reference equality. Splitting it up into elementId and formId would make it safer to add these to effect dependencies.
2. Refactor the Source property (only contains fromUI prop) into a more descriptive paramter: e.g. syncState: bool or triggerRerun: bool.

E.g. the signature could look something like this:

  public setStringValue(
    elementId: string,
    value: string | null,
    formId: string | undefined,
    fragmentId: string | undefined,
    triggerRerun: bool = true,
  )

wdyt?