Skip to content

Enhancement: Extract commonly used functions into a shared file in page commands #6621

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
mkm17 opened this issue Mar 2, 2025 · 6 comments · May be fixed by #6700
Open

Enhancement: Extract commonly used functions into a shared file in page commands #6621

mkm17 opened this issue Mar 2, 2025 · 6 comments · May be fixed by #6700

Comments

@mkm17
Copy link
Contributor

mkm17 commented Mar 2, 2025

Hi, I have noticed that some functions are duplicated in page commands. The idea is to analyze them and extract them into a shared file, like Page.ts.

Here are the functions I have found so far.

_api/site?$select=Id used in

contenttype-add
contenttype-field-remove
contenttype field-set
contenttype-set
page-header-set
site-admin-add

_api/web?$select=Id used in

contenttype-add
contenttype-field-remove
contenttype field-set
contenttype-set
page-header-set
site-admin-add

/checkoutpage used in

page-clientsidewebpart-add
page-header-set
page-section-add

@Adam-it Adam-it added enhancement needs peer review Needs second pair of eyes to review the spec or PR labels Mar 7, 2025
@Adam-it
Copy link
Member

Adam-it commented Mar 7, 2025

@mkm17 awesome suggestion 👍.
Just to clarify before I open it up.
So your idea is to create separate util methods for those functions and just reuse them in the commands you mentioned?
If so do you think we could add links to those price functions to make it super clear what we want to get rid off and replace with a new util method?

@mkm17
Copy link
Contributor Author

mkm17 commented Mar 9, 2025

@Adam-it Yes, the aim is to reduce duplication, as some functions are already exported to the "util" file named Page.ts. I will compile a list of links to certain files.

@Adam-it
Copy link
Member

Adam-it commented Mar 9, 2025

@Adam-it Yes, the aim is to reduce duplication, as some functions are already exported to the "util" file named Page.ts. I will compile a list of links to certain files.

Thanks for the confirmation.
I will open it up. In the mean time we may add those links to make this super clear

@Adam-it Adam-it added help wanted and removed needs peer review Needs second pair of eyes to review the spec or PR labels Mar 9, 2025
@mkm17
Copy link
Contributor Author

mkm17 commented Mar 29, 2025

Hi @Adam-it , you was right, GetSiteId, GetWebId and GetListId can be exported to spo.ts. In spo.ts there is already the getSiteId function but it is using Ms Graph endpoint.

**_api/site?$select=Id

_api/site?$select=Id used in

contenttype-add
contenttype-field-remove
contenttype field-set
contenttype-set
page-header-set
site-admin-add

_api/web?$select=Id used in

contenttype-add
contenttype-field-remove
contenttype field-set
contenttype-set
page-header-set

getListId
contenttype-add
contenttype-field-remove
contenttype-set
listitem-batch-set

And this function from page.ts can be used for /checkoutpage which is used in

page-clientsidewebpart-add
page-header-set
page-section-add

@mkm17
Copy link
Contributor Author

mkm17 commented Apr 16, 2025

Hi @Adam-it what do you think, can I make this change?

@Adam-it
Copy link
Member

Adam-it commented Apr 17, 2025

Hi @Adam-it what do you think, can I make this change?

Totally 👍
Thank you for the ping. I was a bit busy in other projects and I haven't notice this notify before. Sorry for that 🙏
You Rock 🤩

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

Successfully merging a pull request may close this issue.

2 participants