Html Templating for a WIP static site generator, written in Javascript

Question

I wrote this to go in my simple static site generator. it takes any plaintext file (but meant for html) and injects the specified file contents and other data at matching tags. This is my first "big" JavaScript project, and most of my knowledge has come from several-year-old posts on StackExchange, YouTube, and Medium, so I cannot guarantee that I have followed modern conventions.

I have included test code, and It should run as-is however it does require a single index.html file in the same dir to test fileInject().

const fs = require('fs');

var test = `<!-- template:one -->
<!-- template:two -->
<!-- template:three -->
<!-- template:index.html -->
<!-- template:five -->`;

var data = [['one','apple'],['three','orange'],['five','banana']];

// Grab tags from template files. Does not return duplicates.
const getTags = (template) => {
    const regex = /\<\!\-\- template:(?<label>(?:(?!\<\!\-\-).)+) \-\-\>/g;
    var match, matches = [];
    while(match = regex.exec(template)) {
        if(!matches.includes(match)) {
            matches.push([match.groups.label, match[0]]);
        }
    }
    return matches;
}

// Inject data at matching tags. Tag object format: [['key','value'], ...]
// Silent on failure, but mostly because I do not know how to check if template.replace() succeeds or fails. 
const dataInject = (template, tags) => {
    for(let tag of tags) {
        const regex = new RegExp(`\<\!\-\- template:${tag[0]} \-\-\>`, 'g');
        template = template.replace(regex, tag[1]);
    }
    return template;
}

// Find and inject file contents at matching tags.
// Logs errors on failure.
const fileInject = (template) => {
    const tags = getTags(template);
    for(let tag of tags) {
        try{
            fs.accessSync(tag[0]);
            const partial = fs.readFileSync(tag[0]).toString();
            template = template.replace(tag[1], partial);
        } catch(e) { console.log(`error injecting file '${tag[0]}'`) }
    }
    return template;
}

// Individual tests
//console.log(dataInject(test, data));
//console.log(fileInject(test));

//combined tests. Both work, however (if combined) dataInject() should be called first as fileInject() will finish quicker encounter fewer failures.
//console.log(dataInject(fileInject(test), data));
console.log(fileInject(dataInject(test, data)));

Answer 1

Here's a few pointers to help out:

• I see you're using both let and var in there - be consistent and just choose one.
• You're escaping a lot of regular expression characters that don't need to be escaped - it makes the regex less readable. i.e. /\<\!\-\- template:(?<label>(?:(?!\<\!\-\-).)+) \-\-\>/g could be written as /<!-- template:(?<label>(?:(?!<!--).)+) -->/g
• Often it's good to name functions as verbs. i.e. just rename dataInject to injectData and fileInject to injectFile, it reads better.
• Right now, if there was an error reading the template file, you'll always get the same message logged: error injecting file '<file name>'. It doesn't matter if node didn't have permission to access it, or if the file did not exist, you won't get any more information besides that log message. What's worse, you don't rethrow any error, so the program will continue trying to run even though the template has issues. I would just take out the try-catch completely, the original error usually has the most useful information to help debug problems.
• I assume this injection logic is meant to be used by a server. With that use case in mind, you don't want to use the sync versions of the file-system functions - it means the server is unable to handle any other request while it's doing a file operation. Instead of fs.readFileSync(), use fs.readFile(), or even better, fs.promises.readFile().
• The if (!matches.includes(match)) line isn't doing anything. matches is an array of arrays while match is a match object, match will never be inside matches.
• Any reason why data is a list of lists and not an object (or a Map)? Why not just have data equal { one: 'apple', three: 'orange', ... }. If some function needs it in the list-of-list format, it can just call Object.entries() on it.
• Finally, I just want to warn against re-inventing the wheel. There's plenty of great templating systems already out there (i.e. handlebars.js) that come with plenty of great features and built-in XSS protection. If your codebase only needs templates in one or two places, then I can understand preferring to throw together something simple instead of adding a new dependency, but if you're going to be using these templates a lot, it might be better to use a pre-built solution instead of maintaining your own.