Speed and ID concerns converting DOM object into JQuery objects

Question

First of all, I am not using JQuery but Cheerio. I have never actually used JQuery in "real life" -- I started with Cheerio, which is a subset and doesn't actually work in a browser environment.

(Note: I don't have enough reputation here to create tags, somebody might want to tag this question as "cheerio" and delete this sentence...)

Here is my code. Questions are inline:

$ = cheerio.load( self.formPage.toString() );

$( 'form' ).each( function( index, form ){

  // *****************
  // QUESTION:
  // Is this the right way to do this?
  // I am "converting" each found "DOM" object into a "JQuery" object (although
  // it's not real DOM, and it's not real JQuery either).
  // But, if there is no ID set for form, for example, is this _guaranteed_ to always find
  // the right one?
  var $form = $( form );

  // Work out fullAction, which is the form's action relative to the fetched URL
  var action;
  var baseUrl = self.config.url.substr(0, self.config.url.lastIndexOf("/") + 1);
  var action = $form.attr( 'action' );
  var fullAction = require( 'path' ).normalize( baseUrl + action );
  console.log("FULL ACTION:", fullAction );

  stores.workspacesNinjaFormCodes.apiGetQuery( { filters: { workspaceId: self.workspaceId }, skipHardLimitOnQueries: true }, function( err, codesData ){

  //stores.workspacesNinjaFormCodes.dbLayer.select( { conditions: { and: [ { field: 'workspaceId', type: 'eq', value: self.workspaceId } ] } }, { skipHardLimitOnQueries: true }, function( err, codesData ){

    codesData.forEach( function( codeData ){
      var code = codeData.code;
      var dataSubmitHash = {};

      // *****************
      // QUESTION:
      // Is this the right way to do this? I want to find all 'input' fields 
      // in this particular form. So, I pass $form as context and look for
      // all 'input' tags. Is this what you are meant to do?
      // ******************
      $( "input", $form ).each( function( index, inputField ){

        // *****************
        // QUESTION:
        // Once again, will this always work, regadrless of IDs etc.? I am passing `form` as the
        // context.
        // ******************
        var $inputField = $( inputField, form );

        // See if the field name is already assigned
        var fieldValue = $inputField.val() ? $inputField.val() : code;

        // Assign values
        dataSubmitHash[ $inputField.attr('name') ] = fieldValue;
      });


      console.log("dataSubmitHash:", dataSubmitHash );
    });

  });

});

Questions are inline, and they are focused on this: how expensive is the "conversion" of cheerio's "DOM" objects into Cheerio "JQuery" objects? And is it always guarantee to get the right one?

Answer 1

Here's what I did, comments in the code.

$ = cheerio.load(self.formPage.toString());
path = require('path'); //Move out path from the loop to avoid re-requiring it
$('form').each(function (index, form) {

  var $form = $(form);

  // Removed action here. You seem to use it after the next line.

  var baseUrl = self.config.url.substr(0, self.config.url.lastIndexOf("/") + 1);
  var action = $form.attr('action');
  var fullAction = path.normalize(baseUrl + action);

  // In the next block, input is in the context of this form
  // We can pull it out of the loop to avoid fetching it on every iteration
  var input = $('input', form);

  // You don't seem to use fullAction in the next block?

  stores.workspacesNinjaFormCodes.apiGetQuery({
    filters: {
      workspaceId: self.workspaceId
    },
    skipHardLimitOnQueries: true
  }, function (err, codesData) {
    codesData.forEach(function (codeData) {
      var code = codeData.code;
      var dataSubmitHash = {};

      // here's the input earlier
      input.each(function (index, inputField) {

        // Let's keep it readable, place then in variables
        var $inputField = $(inputField, form);
        var inputFieldName = $inputField.attr('name');

        // we could use the || operator to check for the first and use it, or use the second if not first.
        var fieldValue = $inputField.val() || code;

        dataSubmitHash[inputFieldName] = fieldValue
      });

      console.log("dataSubmitHash:", dataSubmitHash)
    })
  })
});

In addition, one should not create functions inside loops. Consider moving them out of the loops.