Skip to content
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

Implement a recipeModule manager #4

Closed
rishabhpoddar opened this issue Oct 13, 2020 · 6 comments
Closed

Implement a recipeModule manager #4

rishabhpoddar opened this issue Oct 13, 2020 · 6 comments
Assignees

Comments

@rishabhpoddar
Copy link
Member

rishabhpoddar commented Oct 13, 2020

Should be responsible for:

  • Provide routing functionality. The algorithm is here.

  • Has a way for any module to get the appInfo if needed.

  • Has an init function that takes:

    • The appInfo
    • recipeList: RecipeModule[]
  • Provides an abstract recipeModule class that modules must extend:

    • Provides methods to asks a module if they can handle the current route (excluding websiteBasePath).
    • Get and manage the module's rId. The rId will be an argument to the constructor.
    • Exposes a function that modules must implement to handle the current route.
  • Recipe modules must be build such that parent recipe's rId can be propagated to them. This is how

@rishabhpoddar rishabhpoddar changed the title Implement a module manager Implement a recipeModule manager Oct 15, 2020
@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Oct 16, 2020

AppInfo in the init function:

{
   appName: string,
   websiteDomain: string,
   apiDomain: string,
   apiBasePath?: string, // Default is `/auth`
   websiteBasePath?: string // Default is `/auth`
}

Edit:

  • @NkxxkN: @makerboiAdi and I discussed this and we decided not to have logoFullURL for the MVP. Maybe in a future version.. So I have edited the above to remove that field.
  • (Update on 10th Oct 2020)If the user has a multi-tenant situation, then they can give the websiteDomain as their random domain, and will override the relevant function in the password reset config. In this case, that function will be used and the value of websiteDomain will be ignored.

@rishabhpoddar
Copy link
Member Author

If a recipe is being built on top of another one, how can its rId be propogated down?

Base recipe

class EmailPassword extends RecipeModule {
	
	static instance: undefined | EmailPassoword

	constructor(options: {}, recipeId) {
		super(recipeId)
	}

	static getInstanceIfDefined() => {
		if (instance === undefined) {
			throw error("...");
		}
		return instance;
	}

	static init(options: {..}): ModuleInterface {
		instance = new EmailPassowrd(options, "emailpassword")
	}
	
}

// feature
class SignInUp extends React.Component {

	getEmailPassowordInstance = () => {
		let instance;
		if (this.props.__internal !== undefined && this.props.__internal.instance !== undefined) {
			instance = this.props.__internal.instance;
		} else {
			instance = EmailPassword.getInstanceIfDefined();
		}
		return instance;
	}
	
	defaultCallSignUp = () => {
		let rId = getEmailPassowordInstance().getRecipeId();
	}	

}

New recipe

class EmailPasswordModified extends RecipeModule {
	
	static instance: undefined | EmailPasswordModified

	emailPasswordInstance: EmailPassowrd;

	constructor(options: {}, recipeId) {
		super(recipeId)
		this.emailPasswordInstance = new EmailPassword(<options specific to this one>, 
				this.getRecipeId())
	}

	static getInstanceIfDefined() => {
		if (instance === undefined) {
			throw error("...");
		}
		return instance;
	}

	static init(options: {..}): ModuleInterface {
		// create a singleton instance of EmailPassword
		instance = new EmailPasswordModified(options, "emailpasswordmodified")
	}

}

// feature
class SignInUpModified extends React.Component {

	getEmailPassowordModifiedInstance = () => {
		let instance;
		if (this.props.__internal !== undefined && this.props.__internal.instance !== undefined) {
			instance = this.props.__internal.instance;
		} else {
			instance = EmailPasswordModified.getInstanceIfDefined();
		}
		return instance;
	}

	render() {
		return (
			<SignInUp
			{...this.props}
			__internal: {
				instance: this.getEmailPassowordModifiedInstance().emailPasswordInstance
				...this.props.__internal
			}
			{...this.props.children}
			/>
		);
	}
}

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Oct 16, 2020

Router: #12

@kant01ne kant01ne self-assigned this Oct 16, 2020
@kant01ne kant01ne mentioned this issue Oct 16, 2020
7 tasks
@kant01ne
Copy link
Contributor

kant01ne commented Oct 19, 2020

Provide routing functionality which:

If 0 modules say yes, then redirects the user to /

We should not actually take care of any redirection, that would break customer's app. We should simply inform the router that we do not are not responsible for this route. The router will take care of managing the rest of the routing accordingly.

New proposition to optimise a little bit and make sure we do not loop over routes if we know already that the rid doesn't match:

Case rid is present:

  • If route does not starts with websiteBasePath, skip
  • Ask all modules if they can handle the current route.
    • rid doesn't match, then skip.
  • Return matching module (only one), or return nothing (router will take care of any redirection itself).

Case no rid is present::

  • Return the first component that matches the route.

@kant01ne
Copy link
Contributor

Maybe we can rename getEmailPassowordInstance and getEmailPassowordModifiedInstance to be getRecipe?
That makes it more consistent across all the features?

@rishabhpoddar
Copy link
Member Author

We should not actually take care of any redirection, that would break customer's app. We should simply inform the router that we do not are not responsible for this route. The router will take care of managing the rest of the routing accordingly.

Make sense! Good catch.

New proposition to optimise a little bit

Makes sense. I have updated my original comment to point to your routing algo.

Maybe we can rename getEmailPassowordInstance and getEmailPassowordModifiedInstance to be getRecipe?
That makes it more consistent across all the features?

How about getRecipeInstanceOrThrow for both? So it will become EmailPassword.getRecipeInstanceOrThrowError() and EmailPasswordModified.getRecipeInstanceOrThrowError().

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

No branches or pull requests

2 participants