0

I have NodeJS program.

In one class, I have various utility methods. One function, safeGithubPush, calls safeString, another func in the same class

module.exports = {

  safeString(stringToCheck) {
    console.log(validator.isAscii(stringToCheck), validator.matches(stringToCheck, /^((\w)*[-.]?(\w)*)*$/))
    return (
      validator.isAscii(stringToCheck) &&
      validator.matches(stringToCheck, /^((\w)*[-.]?(\w)*)*$/)
    );
  },

  safeGithubPush(currentJob) {
    if (
      !currentJob ||
      !currentJob.payload ||
      !currentJob.payload.repoName ||
      !currentJob.payload.repoOwner ||
      !currentJob.payload.branchName
    ) {
      this.logIn(
        currentJob,
        `${'    (sanitize)'.padEnd(15)}failed due to insufficient job definition`
      );
      throw invalidJobDef;
    }

    if (
      this.safeString(currentJob.payload.repoName) &&
      this.safeString(currentJob.payload.repoOwner) &&
      this.safeString(currentJob.payload.branchName)
    ) {
      return true;
    }
    throw invalidJobDef;
  },

} 

While this.logIn(), another func in the utility class, works just fine, I get the error for safeString:

Error caught by first catch: TypeError: this.safeString is not a function

I followed a solution offer by another SO post:

safeString: function(stringToCheck){
...
}

safeGithubPush(currentJob) {
...
if (
    this.safeString(currentJob.payload.repoName) &&
    this.safeString(currentJob.payload.repoOwner) &&
    this.safeString(currentJob.payload.branchName)
    ) {
       return true;
   }
}

But this also gets a, TypeError: this.safeString is not a function.

I'm not using arrow functions, which is the explanation for this error on a different SO post

maddie
  • 1,854
  • 4
  • 30
  • 66
  • You don't need `this` in the second example. Try not to think in terms of Java! – Yan Foto Jan 07 '20 at 15:22
  • Show how you are calling `safeGithubPush`. Are you making a reference to the function somewhere? I wouldn't define these functions within the `exports` object. I'd define them in the file, not attached to an object, and then attach them to `exports` at the very end. You then avoid the `this` issue. – zero298 Jan 07 '20 at 15:23

1 Answers1

1

I don't think the reason is determinable with the code you are currently presenting. It likely has something to do with how you are calling safeGithubPush. If you do something that would change the this binding the this.safeString is going to fail.

const foo = {
  fizz() {
    console.log("fizz");
  },
  buzz() {
    this.fizz();
  }
};

// "this" is correct
foo.buzz();

// "this" has no fizz to call
const myFizz = foo.buzz;
myFizz();

Considering you are attaching these to module.exports I am going to guess that you pull these functions off in a require call and then try to use them bare which makes the problem obvious after looking at my example above:

// Ignore these 2 lines, they let this look like node
const module = {};
const require = () => module.exports;
// Ignore above 2 lines, they let this look like node

// Your module "some.js"
module.exports = {
  safeString(str) {
    return true;
  },
  safeGithubPush(currentJob) {
    if (!this.safeString("some")) {
      throw new Error("Not safe");
    }
    return true;
  }
};

try {
  // Some consumer module that doesn't work
  const {safeGithubPush} = require("./some.js");

  const isItSafe = safeGithubPush();

  console.log(`Safe? ${isItSafe}`);
} catch (err) {
  console.error("Didn't bind right \"this\"");
}

try {
  // Some consumer module that DOES work
  const someModule = require("./some.js");

  const isItSafe = someModule.safeGithubPush();

  console.log(`Safe? ${isItSafe}`);
} catch (err) {
  console.error(err);
}

I would restructure this code. You say these are utility functions which makes me think you don't really want to have to structure them with this in mind.

Instead of attaching them all to module.exports at their definition, define them outside and directly reference the functions you want to use, then attach them to exports so other modules can use the functions:

function safeString(stringToCheck) {
  return true;
}

function safeGithubPush(currentJob) {
  if (!safeString("some")) {
    throw new Error("Not safe");
  }
  return true;
}

module.exports = {
  safeString,
  safeGithubPush
};
zero298
  • 25,467
  • 10
  • 75
  • 100
  • Yeah; simply exporting two functions from the same file doesn't create a single 'this' object that allows you to reference the other export in this manner. The last code fragment from zero298 is the "right" way to do it. If you want to make these functions more formally related to each other, then define them as methods in a class, and export the class. – Jeff Breadner Jan 07 '20 at 15:55
  • This makes it easier for func in the file, but a little more challenging to call utility functions in other classes. Now this (in a diff file) `const workerUtils = require(./utils)....workerUtils.safeString("foobar"); workerUtils.safeGithubPush(job)`, I get type errors – maddie Jan 07 '20 at 18:15
  • @maddie Could you edit your question with the new errors? Did you purge your utility file of `this`? – zero298 Jan 07 '20 at 18:19
  • I did purge my utility file of this! I appreciate you taking another look – maddie Jan 08 '20 at 02:05