Skip to content

web info cleanup #8

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

skrukwa
Copy link

@skrukwa skrukwa commented Jun 17, 2025

problem

Functions in src/utils/sharepoint.rest/web.ts are tightly coupled, lack explicit responsibilities, and call each other circularly which is hard to follow and reason with.

Call stack graph generated with Code2Flow ("module": "es2020", splines="ortho"; deleted) of the unchanged code:

before

  • Note that GetJsonSync can call various other functions in this call graph through getFormDigest during POST requests. However, since all requests in these functions are GET requests, I have omitted this call edge.

core URL resolution logic
(GetSiteUrl & GetFileSiteUrl & GetRestBaseUrl)

  • candidate URL is provided via argument
  • hasGlobalContext -> _spPageContextInfo.webServerRelativeUrl
  • window.location.pathname
  • property of WebInfo .ServerRelativeUrl

core WebInfo resolution logic
(GetWebInfo & GetWebInfoSync)

  • GET request using /_api/site/openWebById('{ID}')?$Select={...WEB_INFO}
  • GET request using /_api/web?$Select={...WEB_INFO}

core WebId resolution logic
(GetWebId & GetWebIdSync)

  • GET request using /_api/web/Id

proposed changes

There are 2 circular call stacks present. The changes proposed do not (should not?) change ANY exported function behaviour.

  1. GetSiteUrl > GetWebInfoSync > GetSiteUrl. Here, if GetSiteUrl is given a GUID, it intentionally calls GetWebInfoSync with siteUrl = null to trigger the "local" resolution of getting an initial siteUrl to make the GetWebInfoSync call with. The proposed solution is to refactor the "local" resolution logic of GetSiteUrl into GetSiteUrlLocally. This way, GetSiteUrl is given a clearer responsibility, and functions that want to resolve a siteUrl locally (possibly using a candidate URL, but NOT using a GUID) can call GetSiteUrlLocally.

  2. GetSiteUrl > GetFileSiteUrl > GetWebIdSync > _getWebIdRequestUrl > GetRestBaseUrl > GetSiteUrl. Here, GetFileSiteUrl needs a url to perform its "brute force" check of its current url. It obtains that by traversing the call stack all the way to GetSiteUrl which just returns makeServerRelativeUrl(normalizeUrl(<url>)). The proposed solution is to keep its logic more contained in its function by simply calling makeServerRelativeUrl(normalizeUrl(<url>)) directly, thus, not giving the impression that GetSiteUrl is performing some sort of circular magic.

    Note: There was also an infinite loop bug in GetFileSiteUrl that has been fixed, and a bug where the site file name may have been popped that was fixed.

Call stack graph after changes:

after

@skrukwa skrukwa marked this pull request as ready for review June 23, 2025 14:28
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.

1 participant