codegouvfr / react-dsfr

🇫🇷 The Frech Government Design system React toolkit
https://react-dsfr.codegouv.studio
MIT License
403 stars 49 forks source link

`only-include-used-icons` may look into the wrong folder #234

Closed sneko closed 6 months ago

sneko commented 6 months ago

Hi @garronej ,

I'm working on a project that fetches multiple repositories in a subfolder, and this folder is .gitignore since it's only for analyses.

Unfortunately the binary only-include-used-icons throws an error trying to find dsfr assets into them:

[Error: ENOENT: no such file or directory, realpath '/xxx/myproject/data/initiatives/ac3a1957-2f3c-4aec-8b61-61d45ab9655f/repositories/a7510318-ebab-4559-b3b9-767277bac8a2/public/dsfr/fonts'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'realpath',
  path: '/xxx/myproject/data/initiatives/ac3a1957-2f3c-4aec-8b61-61d45ab9655f/repositories/a7510318-ebab-4559-b3b9-767277bac8a2/public/dsfr/fonts'
}

I tried to have a look at https://github.com/codegouvfr/react-dsfr/blob/main/src/bin/only-include-used-icons.ts but I don't get which instructions would try to go into nested folders (cwd and getProjectRoot seem to not do that).

If you have any idea, I would appreciate!

garronej commented 6 months ago

Hey @sneko,
How are you doing?
Can you share the link of the repo?
I'll update the code so it works in your usecase. Maybe the best aproach would be to make the script take a argument that point to the source directory but I try to keep the API surface as small as possible. I don't want to shift the burden of configuration to the user when I can figure out myself where I should search. Obviously here I failed to locate the source dir

sneko commented 6 months ago

Good thanks :)

I reproduced with your demo repo: https://github.com/garronej/react-dsfr-next-appdir-demo

It's important to note I'm lucky to see the error, because the third-party repository I was analyzing has DSFR stuff but they committed symbolic links (pointing to nowhere). That's why the binary says no such file or directory, otherwise your script would have work correctly but on the wrong folder.

Here the file https://github.com/betagouv/collectif-objets/blob/main/public/dsfr/dsfr.min.css to make it fail (you can recreate it with ln -s ../nowhere.min.css dsfr.min.css).

You can copy this into your demo repository into react-dsfr-next-appdir-demo/my-subfolder-repo/public/dsfr/dsfr.min.css to perform tests.

Tell me if you prefer a fork.

garronej commented 6 months ago

Try with 1.5.1, should be fixed

sneko commented 6 months ago

It does not work, I tried it on your demo repo with a symlink in a subfolder. According to the commit https://github.com/codegouvfr/react-dsfr/commit/af517455dc340adb9e88a54623da94f9d235367c you added the check is done once candidateFilePaths is computed whereas I think the issue is before the array is built.

I may explore it in the next days.

In all cases, skipping symbolic links in my case will fix the error, but your binary will look for icons of other repositories. Which is fine I guess because:

  1. I download/analyzer third-party repositories at runtime
  2. The usage of only-include-used-icons is during local development or when building (and when building for production the CI/CD has a clean environment so no nested repositories)
  3. Having other icons locally does not affect me that much, I'm fine with it (may be a bit long if thousands of repositories to analyze but it's too specific to my case)
garronej commented 6 months ago

Skipping the symbolic links is what I tried to do but apparently I should have made a mistake.
Please provide me with a step by step reproduction and I'll fix it write away.
Something like:

git clone https://github.com/garronej/react-dsfr-next-appdir-demo  
yarn install
yarn run something
sneko commented 6 months ago

Yes sorry, here it is:

git clone https://github.com/garronej/react-dsfr-next-appdir-demo 
cd react-dsfr-next-appdir-demo
yarn install
mkdir -p data/no-matter-subfolder/public
ln -s ../nowhere.dsfr.min.css data/no-matter-subfolder/public/dsfr.min.css
yarn run predev
garronej commented 6 months ago

thanks a tot for reporting and for the repro! fixed