Skip to content

Commit 78840f5

Browse files
jadeddeltajadeddelta
authored
Merge pull request #3557 from jspsych/parameter-type-error-handling
parameter type mismatch checking + improvements to `ParameterType.SELECT`
2 parents 4811be3 + 8ea4b0b commit 78840f5

4 files changed

Lines changed: 189 additions & 1 deletion

File tree

‎.changeset/eleven-drinks-fail.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"jspsych": patch
3+
---
4+
5+
parameter types will properly be checked in case of type mismatch, along with `ParameterType.SELECT` params properly using the `option` field to check if it is a valid parameter. this will only warn the developer, but in v9 we plan to make it error.

‎packages/jspsych/src/modules/plugins.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export type ParameterInfo = (
5151
array?: boolean;
5252
pretty_name?: string;
5353
default?: any;
54+
options?: any;
5455
};
5556

5657
export type ParameterInfos = Record<string, ParameterInfo>;

‎packages/jspsych/src/timeline/Trial.spec.ts‎

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ describe("Trial", () => {
235235
});
236236

237237
it("respects the `save_trial_parameters` parameter", async () => {
238-
const consoleSpy = jest.spyOn(console, "warn").mockImplementation();
238+
const consoleSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
239239

240240
TestPlugin.setParameterInfos({
241241
stringParameter1: { type: ParameterType.STRING },
@@ -560,6 +560,110 @@ describe("Trial", () => {
560560
});
561561
});
562562

563+
describe("with parameter type mismatches", () => {
564+
//TODO: redo these to expect errors on v9!
565+
let consoleSpy: jest.SpyInstance;
566+
567+
beforeEach(() => {
568+
consoleSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
569+
});
570+
571+
afterEach(() => {
572+
consoleSpy.mockRestore();
573+
});
574+
575+
it("errors on non-boolean values for boolean parameters", async () => {
576+
TestPlugin.setParameterInfos({
577+
boolParameter: { type: ParameterType.BOOL },
578+
});
579+
580+
// this should work:
581+
await createTrial({ type: TestPlugin, boolParameter: true }).run();
582+
583+
// this shouldn't:
584+
await createTrial({ type: TestPlugin, boolParameter: "foo" }).run();
585+
expect(consoleSpy).toHaveBeenCalledWith(
586+
'A non-boolean value (`foo`) was provided for the boolean parameter "boolParameter" in the "test" plugin.'
587+
);
588+
});
589+
590+
it("errors on non-string values for string parameters", async () => {
591+
TestPlugin.setParameterInfos({
592+
stringParameter: { type: ParameterType.STRING },
593+
});
594+
595+
// this should work:
596+
await createTrial({ type: TestPlugin, stringParameter: "foo" }).run();
597+
598+
// this shouldn't:
599+
await createTrial({ type: TestPlugin, stringParameter: 1 }).run();
600+
expect(consoleSpy).toHaveBeenCalledWith(
601+
'A non-string value (`1`) was provided for the parameter "stringParameter" in the "test" plugin.'
602+
);
603+
});
604+
605+
it("errors on non-numeric values for numeric parameters", async () => {
606+
TestPlugin.setParameterInfos({
607+
intParameter: { type: ParameterType.INT },
608+
floatParameter: { type: ParameterType.FLOAT },
609+
});
610+
611+
// this should work:
612+
await createTrial({ type: TestPlugin, intParameter: 1, floatParameter: 1.5 }).run();
613+
614+
// this shouldn't:
615+
await createTrial({ type: TestPlugin, intParameter: "foo", floatParameter: 1.5 }).run();
616+
expect(consoleSpy).toHaveBeenCalledWith(
617+
'A non-numeric value (`foo`) was provided for the numeric parameter "intParameter" in the "test" plugin.'
618+
);
619+
620+
await createTrial({ type: TestPlugin, intParameter: 1, floatParameter: "foo" }).run();
621+
expect(consoleSpy).toHaveBeenCalledWith(
622+
'A non-numeric value (`foo`) was provided for the numeric parameter "floatParameter" in the "test" plugin.'
623+
);
624+
625+
// this should warn but not error (behavior in v9):
626+
await createTrial({ type: TestPlugin, intParameter: 1.5, floatParameter: 1.5 }).run();
627+
expect(consoleSpy).toHaveBeenCalledWith(
628+
`A float value (\`1.5\`) was provided for the integer parameter "intParameter" in the "test" plugin. The value will be truncated to an integer.`
629+
);
630+
});
631+
632+
it("errors on non-function values for function parameters", async () => {
633+
TestPlugin.setParameterInfos({
634+
functionParameter: { type: ParameterType.FUNCTION },
635+
});
636+
637+
// this should work:
638+
await createTrial({ type: TestPlugin, functionParameter: () => {} }).run();
639+
640+
// this shouldn't:
641+
await createTrial({ type: TestPlugin, functionParameter: "foo" }).run();
642+
expect(consoleSpy).toHaveBeenCalledWith(
643+
'A non-function value (`foo`) was provided for the function parameter "functionParameter" in the "test" plugin.'
644+
);
645+
});
646+
647+
it("errors on select parameters with values not in the options array", async () => {
648+
TestPlugin.setParameterInfos({
649+
selectParameter: {
650+
type: ParameterType.SELECT,
651+
options: ["foo", "bar"],
652+
},
653+
});
654+
655+
// this should work:
656+
await createTrial({ type: TestPlugin, selectParameter: "foo" }).run();
657+
658+
// this shouldn't:
659+
660+
await createTrial({ type: TestPlugin, selectParameter: "baz" }).run();
661+
expect(consoleSpy).toHaveBeenCalledWith(
662+
'The value "baz" is not a valid option for the parameter "selectParameter" in the "test" plugin. Valid options are: foo, bar.'
663+
);
664+
});
665+
});
666+
563667
it("respects `default_iti` and `post_trial_gap``", async () => {
564668
dependencies.getDefaultIti.mockReturnValue(100);
565669
TestPlugin.setManualFinishTrialMode();

‎packages/jspsych/src/timeline/Trial.ts‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ export class Trial extends TimelineNode {
360360
for (const [parameterName, parameterConfig] of Object.entries(parameterInfos)) {
361361
const parameterPath = [...parentParameterPath, parameterName];
362362

363+
// evaluate parameter and validate required parameter
363364
let parameterValue = this.getParameterValue(parameterPath, {
364365
evaluateFunctions: parameterConfig.type !== ParameterType.FUNCTION,
365366
replaceResult: (originalResult) => {
@@ -379,6 +380,83 @@ export class Trial extends TimelineNode {
379380
},
380381
});
381382

383+
// TODO: ensure that this throws an error in v9!
384+
// major parameter type validation
385+
if (!parameterConfig.array && parameterValue !== null) {
386+
switch (parameterConfig.type) {
387+
case ParameterType.BOOL:
388+
if (typeof parameterValue !== "boolean") {
389+
const parameterPathString = parameterPathArrayToString(parameterPath);
390+
console.warn(
391+
`A non-boolean value (\`${parameterValue}\`) was provided for the boolean parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
392+
);
393+
}
394+
break;
395+
// @ts-ignore falls through
396+
case ParameterType.KEYS: // "ALL_KEYS", "NO_KEYS", and single key strings are checked here
397+
if (Array.isArray(parameterValue)) break;
398+
case ParameterType.STRING:
399+
case ParameterType.HTML_STRING:
400+
case ParameterType.KEY:
401+
case ParameterType.AUDIO:
402+
case ParameterType.VIDEO:
403+
case ParameterType.IMAGE:
404+
if (typeof parameterValue !== "string") {
405+
const parameterPathString = parameterPathArrayToString(parameterPath);
406+
console.warn(
407+
`A non-string value (\`${parameterValue}\`) was provided for the parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
408+
);
409+
}
410+
break;
411+
case ParameterType.FLOAT:
412+
case ParameterType.INT:
413+
if (typeof parameterValue !== "number") {
414+
const parameterPathString = parameterPathArrayToString(parameterPath);
415+
console.warn(
416+
`A non-numeric value (\`${parameterValue}\`) was provided for the numeric parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
417+
);
418+
}
419+
break;
420+
case ParameterType.FUNCTION:
421+
if (typeof parameterValue !== "function") {
422+
const parameterPathString = parameterPathArrayToString(parameterPath);
423+
console.warn(
424+
`A non-function value (\`${parameterValue}\`) was provided for the function parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
425+
);
426+
}
427+
break;
428+
case ParameterType.SELECT:
429+
if (!parameterConfig.options) {
430+
const parameterPathString = parameterPathArrayToString(parameterPath);
431+
console.warn(
432+
`The "options" array is required for the "select" parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
433+
);
434+
}
435+
}
436+
437+
// truncate floats to integers if the parameter type is INT
438+
if (parameterConfig.type === ParameterType.INT && parameterValue % 1 !== 0) {
439+
const parameterPathString = parameterPathArrayToString(parameterPath);
440+
console.warn(
441+
`A float value (\`${parameterValue}\`) was provided for the integer parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin. The value will be truncated to an integer.`
442+
);
443+
444+
parameterValue = Math.trunc(parameterValue);
445+
}
446+
}
447+
448+
if (parameterConfig.type === ParameterType.SELECT) {
449+
if (!parameterConfig.options.includes(parameterValue)) {
450+
const parameterPathString = parameterPathArrayToString(parameterPath);
451+
console.warn(
452+
`The value "${parameterValue}" is not a valid option for the parameter "${parameterPathString}" in the "${
453+
this.pluginInfo.name
454+
}" plugin. Valid options are: ${parameterConfig.options.join(", ")}.`
455+
);
456+
}
457+
}
458+
459+
// array validation
382460
if (parameterConfig.array && !Array.isArray(parameterValue)) {
383461
const parameterPathString = parameterPathArrayToString(parameterPath);
384462
throw new Error(

0 commit comments

Comments
 (0)