0

I'm creating a basic class in a Node project, and I want to test it using Jest. I'm getting an error that is implying the use of 'strict' mode in the test, which I want to avoid/fix


~/lib/LogRecord.js
module.exports = class LogRecord {
    constructor(level, message, timestamp) {
        this.level = level;
        this.message = message;
        this.timestamp = timestamp ? timestamp : Date.now();
    }

    get level() {
        return this.level;
    }

    get message() {
        return this.message;
    }

    get timestamp() {
        return this.timestamp;
    }
}

I'm testing it with this:

let LogRecord = require('../lib/logRecord');

describe('Test LogRecord functionality', () => {
    test('LogRecord constructor', () => {
        let timestamp = Date.now();
        let logRecord = new LogRecord('INFO', 'Test Message', timestamp);
        expect(logRecord.level).toBe('INFO');
        expect(logRecord.message).toBe('Test Message');
        expect(logRecord.timestamp).toBe(timestamp);
    });
    test('LogRecord is read-only', () => {
        let timestamp = Date.now();
        let logRecord = new LogRecord('INFO', 'Test Message', timestamp);

        logRecord.level = 'WARN'
        logRecord.message = 'New Message'
        logRecord.timestamp = Date.now();

        expect(logRecord.level).toBe('INFO');
        expect(logRecord.message).toBe('Test Message');
        expect(logRecord.timestamp).toBe(timestamp);
    });
});

When I run npm test I get the following error on both of the LogRecord tests:

Test LogRecord functionality › LogRecord constructor

TypeError: Cannot set property level of #<LogRecord> which has only a getter

  1 | module.exports = class LogRecord {
  2 |     constructor(level, message, timestamp) {
> 3 |         this.level = level;
    |         ^
  4 |         this.message = message;
  5 |         this.timestamp = timestamp ? timestamp : Date.now();
  6 |     }

  at new LogRecord (lib/logRecord.js:3:9)
  at Object.test (test/logRecord.test.js:6:25)

Edit - Working class

const data = new WeakMap();

let levelKey = {id:'level'};
let messageKey = {id:'message'};
let timestampKey = {id:'timestamp'};

module.exports = class LogRecord {
    constructor(level, message, timestamp) {
        data.set(levelKey, level);
        data.set(messageKey, message);
        data.set(timestampKey, timestamp ? timestamp : Date.now());
    }

    get level () {
        return data.get(levelKey)
    }

    get message () {
        return data.get(messageKey)
    }

    get timestamp () {
        return data.get(timestampKey)
    }
}
J Lewis
  • 462
  • 1
  • 4
  • 15
  • 1
    I believe es6 modules are in strict mode anyway, can't find the spec, sorry, but I'm pretty sure i read this somewhere. – Logar Jul 31 '18 at 12:24
  • 1
    I was under the impression that I was using CommonJS modules and not ES6 modules, which I did not think were strict by default. Going off [this link](http://imaginativethinking.ca/what-the-heck-is-node-modules-strict-by-default/) it seems that's case, but it might have changed – J Lewis Jul 31 '18 at 12:28
  • 1
    [Here is the spec](http://www.ecma-international.org/ecma-262/6.0/#sec-strict-mode-code) : `All parts of a ClassDeclaration or a ClassExpression are strict mode code.` – Logar Jul 31 '18 at 12:29
  • Interesting link btw, thanks for sharing – Logar Jul 31 '18 at 12:34
  • Ah okay, my mistake then. I've got to admit I don't really know exactly what you're referring to by the transpiled code. I'm just starting out with ES6 and Jest (which is where babel is coming into play?) – J Lewis Jul 31 '18 at 12:35
  • But I've literally just started this project, and that's about all the code I have. I'm going for baby's first TDD :D – J Lewis Jul 31 '18 at 12:36
  • 1
    If you want the property to be read-only then you don't need to test writing it. If you don't want it to be read-only then go ahead and define a setter (or just access the properties directly, this is JavaScript after all). This has nothing to do with strict mode per se. – Jared Smith Jul 31 '18 at 13:00
  • If you could write an answer about making the obj properties read-only while a being valid CommonJS module, that would definitely solve my problem and I can then mark it as answered. Atm the properties are not read only – J Lewis Jul 31 '18 at 13:10
  • 1
    @Logar See [Which ECMAScript 6 features imply strict mode?](https://stackoverflow.com/q/29283935/1048572) – Bergi Jul 31 '18 at 13:36
  • Yup, I also figured it out after reading the specs, thanks – Logar Jul 31 '18 at 13:44

2 Answers2

3

Testing is about making sure that your code does what you think it does. Consider the following class:

class Foo {
  constructor (bar) {
    this._bar = bar;
  }

  get bar () {
    return this._bar;
  }
}

Here bar is read-only, there is no way to set the bar property:

let foo = new Foo('a foo');
foo.bar; // 'a foo'
foo.bar = 'am not'; // TypeError!

The modules question isn't really relevant: as Logar linked in the comments class bodies are always strict mode irregardless.

So if you want a property to be read only, you don't need to worry about writing it. Workflow might look something like this:

  1. Write empty class Foo class Foo {} and construct an instance foo = new Foo()
  2. Write test that checks for bar which fails because we have an empty class
  3. Add constructor parameter and getter
  4. Check that test now passes
  5. Add test to ensure that trying to set bar throws expected error*

If you don't want read-only properties you can just add a setter:

class Foo {
  constructor (bar) {
    this._bar = bar;
  }

  get bar () {
    return this._bar;
  }

  set bar (value) {
    this._bar = value;
  }
}

In which case you'd add a test that sets bar and the reads the altered value back out.

* You might be wondering why this test is here when this behavior is guaranteed by the spec and I would argue that the test is necessary since someone could (transparently to the callers) refactor the class to be an old-school constructor and create a vector for bugs:

// post refactor Foo
const Foo = function Foo(bar) {
  this.bar = bar; // danger! now writable!
};

Hopefully this sort of thing would be caught by a knowledgable reviewer, but I'd write the test anyways.

Update

If what you want is a guaranteed read-only property that you set in the constructor, here is a recipe for such:

const data = new WeakMap();

module.exports = class Foo () {
  constructor (bar) {
    data.set(this, bar);
  }

  get bar () {
    return data.get(this);
  }
};

Because data is not exported no outside code can change it. Attempting to set the bar property of an instance will throw. This is a bit more complicated that just defining an underscore property with getters and setters, but if it's what you want, well... I know this pattern because I've used it.

Update 2

You only create one weakmap per module, not per class or instance. The weakmap stores a unique data entry keyed to individual instances (i.e. this):

const data = new WeakMap();

module.exports = {
  Foo: class Foo () {
    constructor (bar) {
      data.set(this, bar);
    }

    get bar () {
      return data.get(this);
    }
  },

  Bar: class Bar () {
    constructor (prop1, prop2) {

      // for multiple props we'll store an object...
      data.set(this, { prop2, prop1 });
    }

    get prop1 () {
      // ...and retrieve it and access it's props to return
      return data.get(this).prop1;
    }

    get prop2 () {
      return data.get(this).prop2;
    }
  }
};

Note that setting the props with a getter but no setter will still throw...

// in someotherfile.js
const { Foo } = require('path/to/file/with/foo.js');
const foo = new Foo('imma foo');
foo.bar; // 'imma foo'
foo.bar = 'no not really'; // TypeError!

// you can still set random properties that don't have a getter:
foo.baz = 'I do not throw';
foo.baz; // 'I do not throw'
Jared Smith
  • 19,721
  • 5
  • 45
  • 83
  • This is exactly what I'm after, but unfortunately I'm still getting errors due to LogRecord being a CommonJS module. I've updated my post with the read-only tests included, but they're having the same strict mode error – J Lewis Jul 31 '18 at 13:31
  • 2
    Actually the assignment inside the constructor will already throw as there is only a getter on the prototype. Also using the getter will result in a stack overflow anyway. – Bergi Jul 31 '18 at 13:33
  • @JLewis *module format does not matter*. If you provide a getter but not a setter then it will throw when you try to set it. To see what I mean, comment out your getters and run your program. – Jared Smith Jul 31 '18 at 13:33
  • @Bergi has put it better than me, the issue I'm having is that the constructor is throwing an error, I'm not even getting to the read-only aspect of it yet. Taken directly from your example; `let foo = new Foo('a foo');` will throw `Uncaught TypeError: Cannot set property bar of # which has only a getter at new Foo (:3:14) at :1:11` – J Lewis Jul 31 '18 at 13:43
  • @JLewis bergi explained it better than you realize: when you make the assignment in the constructor it sees the getter you defined in the class body and the lack of setter and throws. – Jared Smith Jul 31 '18 at 13:46
  • 1
    @JaredSmith I think OP would like the properties to be initialized within the object creation but readonly from outside the class (and the constructor ?) – Logar Jul 31 '18 at 13:50
  • @Logar only two ways I know of to accomplish that: use an unexported Map/WeakMap to store object data or `Object.defineProperty(this, 'bar', ....)` in the constructor. – Jared Smith Jul 31 '18 at 13:52
  • I've got a very simple one, I think this is working, will post it asap – Logar Jul 31 '18 at 13:53
  • @Logar you can be simple or foolproof but rarely both. Even symbol keys can be gotten back out with `getOwnPropertySymbols`. – Jared Smith Jul 31 '18 at 13:54
  • I agree with that, I just gave it a shot, it doesn't make your answer less relevant anyway – Logar Jul 31 '18 at 13:55
  • 1
    @Logar I went ahead and added the weakmap recipe to my answer. – Jared Smith Jul 31 '18 at 14:00
  • Currently trying to figure out how best to use this with multiple properties, but I reckon it's what I'm after – J Lewis Jul 31 '18 at 14:14
  • Interestingly `logRecord.level = 'NEW VAL'` doesn't seem to be throwing anything, but I need to check my implementation – J Lewis Jul 31 '18 at 14:18
  • Yeah it's not throwing anything when I test it manually in chrome, but it's also definitely read-only, so I'm not sure it's a huge problem – J Lewis Jul 31 '18 at 14:20
  • I figured making key objects was better than creating many weakmaps, so I've ended up with `let levelKey = {id:'level'};`, `data.set(levelKey, level);`, `return data.get(levelKey)`. Worth noting that trying to set logRecord.level definitely does not throw an error – J Lewis Jul 31 '18 at 14:32
  • @JLewis you only create one weakmap, and your intuition for multiple keys is correct see my update #2. – Jared Smith Jul 31 '18 at 15:42
2

If you want your properties to be read only after object initialization, you can use Object.freeze in the constructor, and remove your getters :

class LogRecord {
    constructor(level, message, timestamp) {
        this.level = level;
        this.message = message;
        this.timestamp = timestamp ? timestamp : Date.now();
        Object.freeze(this);
    }
}

But this will freeze all of your object's properties. You won't be able to modify, remove, or add any after that. Didn't dive too deep into this so it may have some flaws as well

Logar
  • 1,248
  • 9
  • 17