-
Notifications
You must be signed in to change notification settings - Fork 89
feat: recursive plugin dependency resolution #1005
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
feat: recursive plugin dependency resolution #1005
Conversation
- Adds tests - Fixes issue with plugin config override allowing modification of non-public properties - Adds support for ts-mocha
|
✅ No documentation updates required. |
test/plugins/plugins.test.ts
Outdated
| string[], | ||
| string[] | ||
| ][] = [ | ||
| // [fnOverride, apiOverride, plugins, expectedFnStack, expectedApiStack] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if these were objects.
| it("error", async () => { | ||
| try { | ||
| const plugin = pluginFactory({ | ||
| identifier: "plugin1", | ||
| overrideFunctions: false, | ||
| overrideApis: false, | ||
| }); | ||
| plugin.routeHandlers = () => ({ status: "ERROR", message: "error" }); | ||
|
|
||
| STExpress.init({ | ||
| ...partialSupertokensConfig, | ||
| recipeList: [recipeFactory({ overrideFunctions: false, overrideApis: false })], | ||
| experimental: { | ||
| plugins: [plugin], | ||
| }, | ||
| }); | ||
|
|
||
| const stInstance = SuperTokens.getInstanceOrThrowError(); | ||
|
|
||
| const resObject = new DummyResponse(); | ||
| const res = await stInstance.middleware(new DummyRequest(), resObject, {} as UserContext); | ||
| } catch (err) { | ||
| assert.strictEqual(err.message, "error"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test structure has an issue with error handling. The error is thrown during initialization when STExpress.init() is called with the plugin that returns an error status, not during middleware execution. To properly test this error case, the try-catch block should wrap the initialization code instead:
it("error", async () => {
const plugin = pluginFactory({
identifier: "plugin1",
overrideFunctions: false,
overrideApis: false,
});
plugin.routeHandlers = () => ({ status: "ERROR", message: "error" });
try {
STExpress.init({
...partialSupertokensConfig,
recipeList: [recipeFactory({ overrideFunctions: false, overrideApis: false })],
experimental: {
plugins: [plugin],
},
});
assert.fail("Should have thrown an error");
} catch (err) {
assert.strictEqual(err.message, "error");
}
});This ensures the test is actually verifying the expected error behavior.
| it("error", async () => { | |
| try { | |
| const plugin = pluginFactory({ | |
| identifier: "plugin1", | |
| overrideFunctions: false, | |
| overrideApis: false, | |
| }); | |
| plugin.routeHandlers = () => ({ status: "ERROR", message: "error" }); | |
| STExpress.init({ | |
| ...partialSupertokensConfig, | |
| recipeList: [recipeFactory({ overrideFunctions: false, overrideApis: false })], | |
| experimental: { | |
| plugins: [plugin], | |
| }, | |
| }); | |
| const stInstance = SuperTokens.getInstanceOrThrowError(); | |
| const resObject = new DummyResponse(); | |
| const res = await stInstance.middleware(new DummyRequest(), resObject, {} as UserContext); | |
| } catch (err) { | |
| assert.strictEqual(err.message, "error"); | |
| } | |
| it("error", async () => { | |
| const plugin = pluginFactory({ | |
| identifier: "plugin1", | |
| overrideFunctions: false, | |
| overrideApis: false, | |
| }); | |
| plugin.routeHandlers = () => ({ status: "ERROR", message: "error" }); | |
| try { | |
| STExpress.init({ | |
| ...partialSupertokensConfig, | |
| recipeList: [recipeFactory({ overrideFunctions: false, overrideApis: false })], | |
| experimental: { | |
| plugins: [plugin], | |
| }, | |
| }); | |
| assert.fail("Should have thrown an error"); | |
| } catch (err) { | |
| assert.strictEqual(err.message, "error"); | |
| } | |
| }); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Summary of change
Related issues
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)
Checklist for important updates
coreDriverInterfaceSupported.jsonfile has been updated (if needed)lib/ts/version.tsfrontendDriverInterfaceSupported.jsonfile has been updated (if needed)package.jsonpackage-lock.jsonlib/ts/version.tsnpm run build-prettyrecipe/thirdparty/providers/configUtils.tsfile,createProviderfunction.git tag) in the formatvX.Y.Z, and then find the latest branch (git branch --all) whoseX.Yis greater than the latest released tag.add-ts-no-check.jsfile to include thatsomeFunc: function () {..}).exportsinpackage.jsonRemaining TODOs for this PR