feat(clerk-js): Add organization basic resources by igneel64 · Pull Request #57 · clerk/javascript

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 · 2022-02-22T12:11:41Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

First iteration of the organizations base resources:

Organization
OrganizationMembership
OrganizationInvitation

SokratisVidros

~~@igneel64 As discussed we should align all models to use this._baseX(X=Post,Put, etc...) method for XHR requests.~~

Type fixes and sign-in loading screen

nikosdouvlis · 2022-03-01T08:44:04Z

+ })
+ .then(res => {
+ const organizationsJSON = res.payload
+ ?.response as unknown as OrganizationJSON[];


❓ Just asking, why is the type assertion needed here?
I'd expect that we wouldn't need it because of the generic param in line 48:
.request<OrganizationJSON[]>({
Is this type broken?

nikosdouvlis · 2022-03-01T09:00:34Z

+ this.fromJSON(data);
+ }
+
+ static async create(name: string): Promise<OrganizationResource> {


❓ IIRC, all resources use the this.basePost | this.baseGet | ... methods that retrieve a json and return the instantiated resource. Why did we decide to use the static BaseResource._fetch here?
To be honest, I'm not a huge fan of the this.basePost approach, but if we don't have any specific reasons to do otherwise, I think it'd be better to remain consistent and tackle them all at once in a different PR

@nikosdouvlis
Discussed offline.

… attribute

SokratisVidros · 2022-03-02T15:59:07Z

 this.#fapiClient.onAfterResponse(callback);
 }

+ __unstable_inviteMember = async (


❓ @igneel64 Can you please elaborate why do we consider this an unstable method at this point?

❓ A follow up question, does this method invite a member or creates an org? Something seems to be missing.

@SokratisVidros , thanks for the comment.
The use of the __unstable prefix is intended here. Currently we cannot expose methods on clerk-react from internal clerk-js resources. The only way this can be done is through the Clerk interface.
The same interface though is exposed globally. That means that adding a method there, it is also exposed in window.Clerk.methodName.
Since adding methods indiscriminately on the global Clerk object is not the optimal outcome, for now we are adding to those methods the __unstable prefix to prevent misuse.
(Also this method just invites a member on an organization. The organizations can be retrieved from the useOrganizations hook.)

SokratisVidros · 2022-03-02T16:18:05Z

+ const clerk = useClerk();
+
+ return {
+ createOrganization: clerk.createOrganization,


🔧 Ditto about listing orgs.

…mechanism alignment Fix state clearing of organizationInvitationToken

SokratisVidros

💯

igneel64 requested review from SokratisVidros, alex-ntousias, gkats and nikosdouvlis February 22, 2022 12:11

igneel64 marked this pull request as draft February 22, 2022 12:11

feat(clerk-js): Add organization basic resources
09f9012

igneel64 force-pushed the organization_clerk_js branch from 26c5337 to 09f9012Compare February 22, 2022 13:29

feat(clerk-js): Basic organization data shape tests
0ca9a31

SokratisVidros reviewed Feb 23, 2022
View reviewed changes

SokratisVidros approved these changes Feb 23, 2022
View reviewed changes

igneel64 added 4 commits February 24, 2022 12:36

feat(clerk-js): Invitation flow draft
d6faaab

feat(clerk-js): SignUp with organization invitation flow draft
2a9edbd

feat(clerk-js): Sign up next draft and fixes
e2eef78

feat(clerk-js): Add more attributes on organization models
af010ba
Type fixes and sign-in loading screen

SokratisVidros approved these changes Mar 1, 2022
View reviewed changes

nikosdouvlis reviewed Mar 1, 2022
View reviewed changes

igneel64 added 2 commits March 1, 2022 16:58

feat(clerk-js): Add useOrganization hook
480c422

feat(clerk-react,clerk-js): Add useOrganization hook using __unstable…
1635132
… attribute

igneel64 force-pushed the organization_clerk_js branch from 2e21552 to 1635132Compare March 2, 2022 09:35

igneel64 marked this pull request as ready for review March 2, 2022 09:50

Merge branch 'main' into organization_clerk_js
55cf7c4

igneel64 changed the title ~~WIP: feat(clerk-js): Add organization basic resources~~feat(clerk-js): Add organization basic resourcesMar 2, 2022

SokratisVidros suggested changes Mar 2, 2022
View reviewed changes

igneel64 force-pushed the organization_clerk_js branch from 087735f to 080c50bCompare March 3, 2022 09:37

SokratisVidros reviewed Mar 3, 2022
View reviewed changes

Comment threadpackages/react/src/hooks/useOrganizations.ts Outdated

feat(clerk-js,clerk-react): GetOrganization/s hook methods, fetching …
fc11087
…mechanism alignment Fix state clearing of organizationInvitationToken

igneel64 force-pushed the organization_clerk_js branch from 080c50b to fc11087Compare March 3, 2022 12:03

igneel64 requested a review from SokratisVidros March 3, 2022 12:03

SokratisVidros approved these changes Mar 3, 2022
View reviewed changes

igneel64 merged commit c0f1a6d into mainMar 3, 2022

igneel64 deleted the organization_clerk_js branch March 3, 2022 13:04

github-actionsBot locked as resolved and limited conversation to collaborators Jun 11, 2023