(Feature) Use backstop.js instead of backstop.json by default

[klodoma]

Hi @garris

Did you ever considered using a backstop.js(by default) instead of the backstop.json.

I know one can use it with backstop test --docker --config=backstop.js but I think it would be more efficient for the future. I use this for all projects where I use backstopjs.

Advantages:

• you can add comments to config file
• blocks and be out-commented
• code can be generated dynamically
• more readable
• zillions of double quotes disappear in the config file

Disadvantages:

• ?

Here is a full example of "kind of" what I mean and how I would use it. https://github.com/klodoma/backstopjs-debug/tree/feature/js-config

[garris]

Yes. I would consider changing this convention. It would require updating the docs and some of the scaffolding code.

My only concern would be that it adds some (probably minor) complexity for beginners or QA engineers with limited JS understanding.

[maxfenton]

Could the default backstop test check for either of those files existing? As it stands I have to run backstop test --config=backstop.config.js every time. It seems like a check for backstop.json that falls back to backstop.js would be a nice improvement.

[garris]

I believe it already does this. If you run the sanity check it the docs it should work like that.

[maxfenton]

@garris I think it does if it’s part of a build process but the CLI version does not. I might be wrong, and I'm not sure what I would need to do to change https://github.com/garris/BackstopJS/blob/f0651dc5439cebc1445a611115b7c16f4d2074a4/cli/index.js#L15 enough to make a PR.

My working process is to npm install -g backstopjs and then add a backstop.js configuration to each project directory and locally run backstop with that --config flag.

[maxfenton]

Just wanted to follow up about the comment from August. BackstopJS does not have any kind of fallback system for config files names and only looks for backstop.json in cli/index.js

Adding an fs call to this seems like it might cause headaches between different node versions, so I understand the hesitation to add an enum of config file names.

[dgrebb]

This is a great patch-package use case. Given this value will likely not change over time.

1. npm i -D patch-package
2. add "postinstall": "patch-package" to package.json scripts
3. make a patches directory in project root
4. add the provided patch below
5. run npm i
6. set it and forget it!

diff --git a/node_modules/backstopjs/cli/index.js b/node_modules/backstopjs/cli/index.js
index 893ab7f..51a433e 100755
--- a/node_modules/backstopjs/cli/index.js
+++ b/node_modules/backstopjs/cli/index.js
@@ -12,7 +12,7 @@ function main () {
     boolean: ['h', 'help', 'v', 'version', 'i', 'docker'],
     string: ['config'],
     default: {
-      config: 'backstop.json'
+      config: 'backstop.js'
     }
   });

backstopjs+6.3.23.patch

[maxfenton]

@dgrebb Thank you. patch-package is new to me and very cool. I have Backstop installed globally and run it from outside the project's docker container, so I might not be able to patch-package, but now I see that I could just update this line in ~/.nvm/versions/node/v18.17.1/bin/backstop for my own convenience.

[maxfenton]

I'm going to see if an inelegant change to makeConfig.js works better, too:

    } else if (config.backstopConfigFileName) {
      // Test the existence of an array of files and settle on one that exists
      config.backstopConfigFileName = require.resolve(config.backstopConfigFileName) ||
        require.resolve(path.join(config.projectPath, 'backstop.json')) ||
        require.resolve(path.join(config.projectPath, 'backstop.config.json')) ||
        require.resolve(path.join(config.projectPath, 'backstop.js')) ||
        require.resolve(path.join(config.projectPath, 'backstop.config.js'));

      // Remove from cache config content
      delete require.cache[require.resolve(config.backstopConfigFileName)];
      console.log('Loading config: ', config.backstopConfigFileName, '\n');
      userConfig = require(config.backstopConfigFileName);
    }

[dgrebb]

@maxfenton one could also fork the repo, make these changes, and npm install -g maxfenton/backstopjs#master. The change would be permanent as long as it's installed from that fork.

See other variations of installing an npm package from GitHub.

[klodoma]

@garris

I just had a look at makeConfig.js based on the @maxfenton proposal and I am wondering if options.backstopConfigFilePath || options.configPath is not obsolete and could be removed? I assume if someone want to use a different config file, it will use the --config parameter.,

https://github.com/garris/BackstopJS/blame/6d0e5fe42e15f6fe8468dced56087bf6d29cbbba/core/util/makeConfig.js#L17

[solomonhawk]

I mentioned this in the associated PR #1566: https://github.com/cosmiconfig/cosmiconfig is a fairly mature and standardized way to handle adding this kind of config file flexibility.