New plugin: Survey

[becky-gilbert]

New survey plugin that uses SurveyJS under the hood. Features:

• several supported survey question types: text, multi-choice, multi-select, likert (matrix), drop-down, html content (no response)
• allows for arbitrary combinations of different question types across pages
• ability to go backwards and forwards through pages
• see/edit answers on previous pages
• response validation
• question and choice order randomization

This is a work in progress. I'm creating the PR now so that I can get some implementation feedback/guidance.

[changeset-bot]

🦋 Changeset detected

Latest commit: dee2287

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@jspsych/plugin-survey	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[chrisbrickhouse]

This looks great! Just a couple comments to think about

[becky-gilbert]

Thanks very much for the feedback @chrisbrickhouse!

[becky-gilbert]

Hey @bjoluc, @jodeleeuw, @chrisbrickhouse (and anyone else!) - I could use some guidance on implementation decisions with this plugin. I got this basic version working just so I could figure out how to use SurveyJS and make sure it has the features we need. But now it needs refactoring.

The question-type-specific parameters (nested in pages) in the parameter info are all optional, and I'm not sure how to make some of them required based on the question type. One option is to leave them all optional in the parameter info and then do parameter checks in each of the functions that sets up the question type (e.g. throw an error there if a required parameter is not provided). But what might be better is if I could define separate parameter info types for each question type. Not sure how to do that, and if it's worth the effort?

I'm also aware that I've gone for the naïve solution of putting all of the question-type-specific set-up code in functions in the main trial method, and perhaps it would be better for the general trial code and question-type-specific code to to be more modular. We could have a general question class that each question type uses (I'm not sure how to do that?). Or maybe an easier solution is just to move the question-type functions into separate methods in the plugin class, to make the code clearer and to make it easier to add/modify question types in the future.

I'd appreciate any thoughts on these implementation decisions. And if you think I should set up a question class and/or separate parameter info types for each question type, please do let me know if you have any tips on how to do this!

[jodeleeuw]

One option is to leave them all optional in the parameter info and then do parameter checks in each of the functions that sets up the question type (e.g. throw an error there if a required parameter is not provided).

This seems reasonable to me for now. I still don't fully understand the black magic that @bjoluc conjured up to make all the typing work for these parameters, so I'm not sure if there's a better solution.

I imagine there's something where we could define an Interface for each question type, and then nest those interfaces conditional on a question type, but I dunno how to make that work 😓. Maybe it would require moving to explicit constructors for each question? Too much for now...

Or maybe an easier solution is just to move the question-type functions into separate methods in the plugin class, to make the code clearer and to make it easier to add/modify question types in the future.

I like this solution! I used a lot of private functions in the code for the sketchpad plugin and found it much easier to work with.

[becky-gilbert]

I still don't fully understand the black magic that @bjoluc conjured up to make all the typing work for these parameters

Glad it's not just me! 😆

I imagine there's something where we could define an Interface for each question type, and then nest those interfaces conditional on a question type, but I dunno how to make that work 😓. Maybe it would require moving to explicit constructors for each question? Too much for now...

Yes this is what I was trying to figure out, but maybe we can just focus on getting a reasonable-if-not-ideal version working for now, given the time constraints, and then come back to refactoring later. It would be cool to actually understand how to do this but I'm aware that I don't have loads of time to figure it out.

I like this solution! I used a lot of private functions in the code for the sketchpad plugin and found it much easier to work with.

Great, thanks for the tip and link to your code!

[bjoluc]

One option is to leave them all optional in the parameter info and then do parameter checks in each of the functions that sets up the question type (e.g. throw an error there if a required parameter is not provided).

This seems reasonable to me for now. I still don't fully understand the black magic that @bjoluc conjured up to make all the typing work for these parameters, so I'm not sure if there's a better solution.

Right now, the magic (let's not call it black 😜) is just a shorthand to type the trial parameter for usage in our trial methods. If it's about finding a typing solution on our end, we can simply modify the type that we annotate the trial parameter with (the proper TypeScript constructs here would be union types and type guards). But I guess the question is rather about validation in the jsPsych core at runtime? If so, I don't think this use case is common enough to build something heavy like "complex union types" into the core (especially not before refactoring anything there). I agree with both of you about having specific validation logic in the plugin. In case manual validation gets to heavy/complex, something like JSON schema and ajv could also be an option.

I imagine there's something where we could define an Interface for each question type, and then nest those interfaces conditional on a question type, but I dunno how to make that work sweat. Maybe it would require moving to explicit constructors for each question? Too much for now...

Typescript-wise, that would be union types and type guards – if you're after that, I'm happy to set it up in a free moment – but it would just be for our pleasure then...

Or maybe an easier solution is just to move the question-type functions into separate methods in the plugin class, to make the code clearer and to make it easier to add/modify question types in the future.

100%, I wouldn't suggest anything else :)

Other than that, I think the plugin looks very good already! Let me know if there's anything I should help with!

[becky-gilbert]

Wonderful, thanks @bjoluc!

[chrisbrickhouse]

I think @bjoluc said most of what I was thinking. I spent some time today looking into type unions and...yikes... They seem like a cool feature of TS that might be useful in another context, but using them here strikes me as an anti-pattern. My main concern is that they increase the learning curve for minimal gain. More esoteric code means that there are fewer people who can integrate upstream changes or contribute downstream changes.

move the question-type functions into separate methods in the plugin class

I think this modular solution would be more maintainable. Upstream changes may introduce new form items or users might want to add their own custom ones. I think making those changes will be more straightforward if these were implemented as methods rather than type unions since hackers are probably more familiar with objects than strict typing.

[becky-gilbert]

Thanks for your thoughts @chrisbrickhouse!

I spent some time today looking into type unions and...yikes... They seem like a cool feature of TS that might be useful in another context, but using them here strikes me as an anti-pattern. My main concern is that they increase the learning curve for minimal gain. More esoteric code means that there are fewer people who can integrate upstream changes or contribute downstream changes.

I think making those changes will be more straightforward if these were implemented as methods rather than type unions since hackers are probably more familiar with objects than strict typing.

I hadn't thought of it this way but now I totally see your point. I think I was too focused on finding the "right" (ES6/TS/OOP) way of setting up this plugin that I started to forget about readability and user-friendliness.

[becky-gilbert]

I'd love to get some feedback on how this new survey plugin should store the data. Sticking with jsPsych conventions, the response data is stored in a response object, with each key being a question name/ID and value being the response. However, this plugin can present multiple pages of questions, so I changed the default question naming from just the question number ("Q0", "Q1" etc.) to be a combination of the page number and question number ("p0_q0", "p0_q1", "p1_q0" etc.). I figured it would be simpler to store all of the responses in one object rather than separating the responses according to pages.

Also unlike the existing survey-* plugins, this one can have a mix of different question types, which means that the response data is not necessarily a consistent type.

Here's an example of what the data currently looks like:

I'm wondering if this is ok as it is, or if it would help to also include the mapping between question name/ID and question type (multi-choice, text, etc.) in the data. I thought about adding this information into the response object, but that would mean that each question in the response object would also need to be an object, which would make the data more complicated and difficult to parse. So then I thought a better option might be to have a separate property in the data for question types, so that the trial data would look something like this:

{
  "rt": 10290,
  "response": {
    "p0_q0": null,
    "p0_q1": "hi",
    "p0_q2": "",
    "p1_q0": "option 6",
    "p1_q1": [
      "option 6",
      "option 4"
    ],
  },
"question_type": {
    "p0_q0": "html",
    "p0_q1": "text",
    "p0_q2": "text",
    "p1_q0": "multi-choice",
    "p1_q1": "multi-select",
  },
  "trial_type": "survey",
  // ...
}

Any thoughts/suggestions?

[becky-gilbert]

since we have for (const ... of ...) loops, the functional-style forEach does no longer provide any benefits.

Ok I can change this, even though I'm a fan of functional-style programming 😃

[becky-gilbert]

I know I'm a little late to the party, but I remember when I tried out SurveyJS, I liked the flat keying-responses-by-name data format a lot. I think allowing people to name their questions might be a good thing, as it enables meaningful result data. If it were up to me, I would probably keep the flat result format and add an optional name parameter to each question type that, if set, replaces the p0_q0 key in the result data.

I tend to agree with this but am happy to be overruled.

[bjoluc]

Ok I can change this, even though I'm a fan of functional-style programming 😃

Totally agree, FP is great! I just wouldn't use it in this particular scenario, i.e. to write imperative code in a more complex way. Some places with great FP potential:

• filter:

  const invalid_params = [];
  supplied_params.forEach((param: string) => {
    if (!(optional.includes(param) || required.includes(param))) {
      invalid_params.push(param);
    }
  });

  =>

  const invalid_params = supplied_params.filter(
    (param) => !(optional.includes(param) || required.includes(param))
  );

• map:

  question.columns = [];
  params.options.forEach((opt: string, ind: number) => {
    question.columns.push({ value: ind, text: opt });
  });

  =>

  question.columns = params.options.map((opt: string, ind: number) => ({
    value: ind,
    text: opt,
  }));

[becky-gilbert]

Wow @bjoluc, your refactoring is SUPER helpful - thanks!! And sorry that you've had to re-write pretty much everything... 😬

[bjoluc]

And sorry that you've had to re-write pretty much everything...

Not at all, the code and structure before was just fine! I just like to use refactoring as a means of getting the gist of code I get to work on...