Skip to content

Comments

Fix approve command with custom fileNameFormatter#532

Open
d8corp wants to merge 1 commit intooblador:masterfrom
d8corp:fix-approve
Open

Fix approve command with custom fileNameFormatter#532
d8corp wants to merge 1 commit intooblador:masterfrom
d8corp:fix-approve

Conversation

@d8corp
Copy link

@d8corp d8corp commented Jan 28, 2025

No description provided.

@vestrel00
Copy link

vestrel00 commented Apr 1, 2025

Hey, thanks for this! I tried out the changes locally and it seems to work. However, other folders within the reference folder is getting deleted.

Here's my loki.config.js,

module.exports = {
  configurations: {
    'ios.iphone16': {
      target: 'ios.simulator',
    },
    'android.pixel6a': {
      target: 'android.emulator',
    },
  },
  fileNameFormatter: ({ configurationName, kind, story, parameters }) =>
    `${configurationName}/${kind}_${story}`,
};

Running yarn loki update produces reference images in the following file hierarchy,

  • .loki/reference/
    • ios.iphone16/
    • android.pixel6a/

If I make some changes such that running yarn loki test will fail (ios first), running yarn loki approve updates the ios.iphone16/ correctly BUT the android.pixel6a/ gets deleted.


UPDATE:

It seems like this behavior is unrelated to the changes in this PR. The "issue" is caused by this block of existing code;

fs.emptyDirSync(referenceDir);
fs.ensureDirSync(referenceDir);

files.forEach((file) =>
  fs.moveSync(path.join(outputDir, file), path.join(referenceDir, file))
);

In the case of the fileNameFormatter that I'm using, I should be emptying and moving files into the dir with ${configurationName} within the referenceDir instead of the referenceDir itself.


UPDATE 2:

I'm able to workaround this "issue" by specifying the output and reference dirs; yarn loki approve --output="./.loki/current/ios.iphone16/" --reference="./.loki/reference/ios.iphone16/"


UPDATE 3:

Specifying the dirs every time is a bit cumbersome. So, I added a new required cli arg --configurationName to ensure that the approve script only applies to a single configuration (i.e. ios.iphone16 or android.pixel6a).

The command I'm using is now yarn loki approve --configurationName="ios.iphone16".

Here's the code I'm using in case others want to use them; code.txt

The code changes I made are rudimentary. Nothing fancy or proper. Just something that works for my setup. Ideally, the approve script would work without having to specify additional args such as the --configurationName that I added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants