generator icon indicating copy to clipboard operation
generator copied to clipboard

`noOverwriteGlobs` will not work in majority of cases in templates using react

Open derberg opened this issue 4 months ago • 10 comments

After we introduced react engine for templates rendering, this code was not changed

async shouldOverwriteFile(filePath) {
    if (!Array.isArray(this.noOverwriteGlobs)) return true;
    const fileExists = await exists(path.resolve(this.targetDir, filePath));
    if (!fileExists) return true;

    return !this.noOverwriteGlobs.some(globExp => minimatch(filePath, globExp));
  }

the problem is that input here, filePath is always the filePath that includes a file name as it is called in the template.

In nunjuck engine things are easy, we practice to name template files exactly as they are named after they are rendered in output. So for example in html-template we would call a file in template directory by: index.html

In react, you have more flexibility, you can follow the same approach as in nunjuck engine, have separate files templated with react, but you will call the same file I mentioned above index.html.js

Problem

So the code I mentioned above will never work in above mentioned cases as at start the filePath will be index.html.js. When I use generator in CLI for example, I want to pass --no-overwrite="index.html" as I want to make sure it do not overwrite index.html, I do not want to remember to ignore index.html.js. And even if I remember, it will not work as const fileExists = await exists(path.resolve(this.targetDir, filePath)); will anyway return true and override my file cause index.html.js do not exists in output.

Solution

I think a bigger refactor is needed

  • noOverwriteGlobs as it is, should accept only patterns/names that I have in output
  • shouldOverwriteFile should not rely on filenames from template. Fix is not that easy as just remove .js from the filename as can be that template just contains index.js for example.

Some investigation is needed. Probable we should move away from generateFile the usage of shouldOverwriteFile and move it to renderAndWriteToFile where for nunjucks we do it old way and for react, we grab final name of file in output somehow from react 🤔

derberg avatar Feb 22 '24 19:02 derberg