chore: Remove unused files and exports

maschad commented 7 months ago

Running knip reports the following

unused files

unused exports

unused export types

I suspect there may be some more configuration to be done to get this to be more accurate.

Once #2024 is resolved this can be done.

arboleya commented 7 months ago

What is considered an unused export?

For instance, we don't want to stop exporting constants meant to be used externally.

maschad commented 7 months ago

Agreed @arboleya I think we will have to add a bit of manual configuration around the unused export issue type to ensure we don't remove those. It will be quite a bit of grunt work upfront unfortunately.

arboleya commented 7 months ago

If I'm following the idea here (and on https://github.com/FuelLabs/fuels-ts/pull/2026), the introduction of knip is to automate and make up for [manual] things we can forget and may go unnoticed. However, considering the configuration exceptions will also be manual, can go outdated, and be somehow forgotten, will it still be an improvement over the current scenario?

maschad commented 7 months ago

That's a good point, but I don't think that it applies to unused files or unresolved imports or duplicate exports, as we shouldn't seek to introduce anymore of those.

The manual configuration applies primarily to the unused exports issue type , but considering that in an ideal scenario, all the exported constants that would be consumed by external libraries would be distributed across constants.ts files, we could then rationalize that other constants would unused. As you rightly point out this will still be manual but I think it reduces the overall manual overhead considering the points mentioned above.

webpro commented 6 months ago

Apologies in advance for my unsolicited feedback here.

What is considered an unused export?

For instance, we don't want to stop exporting constants meant to be used externally.

By default, exports of entry files are not reported as unused (this is configurable though). In other words, in this case, you could add constants.ts as an entry file and call it a day.

However, considering the configuration exceptions will also be manual, can go outdated, and be somehow forgotten, will it still be an improvement over the current scenario?

This is a great point. For example, ESLint disable-line comments, rules or even plugins might no longer be necessary at some point too. This type of maintenance is almost inevitable. Knip tries to be as unobtrusive as possible. For instance, as a solution for the same issue, exports can be JSDoc-tagged @public, serving as (internal) documentation at the same time.

AMA! There's some ways to go about configuration, I'm sure there's a way to knip it ✂️

maschad commented 5 months ago

Thanks for creating this awesome library @webpro 🚀 , and also thanks for your input, really appreciate your direction on this 😄 . I think right now all the reported unused files and exports are misreported for us, and so the configuration would be disproportionately more than the size savings.

For instance BYTES_32 and a number of other constants are reported as Duplicate exports even though they aren't duplicates but rather re-used.
All these ErrorCodes are reported as unused as well, and we wouldn't want to set that file as our entry file, rather if it could determine that the fact it's exported via the index entry point that would be useful.
All the reported unused files are actually configuration files such as vitest test configs for example.

I think we can revisit this issue in the future as knip matures.

webpro commented 5 months ago

Thanks a lot for the feedback, it was very helpful. Full disclosure:

Not trying to push anything here, just know that since v5.17 with a lot less configuration the results are a lot better:

{
  "workspaces": {
    "*/*": {
      "entry": ["src/{index,main}.{ts,tsx}", "fuels.config*.ts"]
    }
  },
  "ignoreDependencies": [
    "@fuel-ts/.+",
    "@graphql-codegen/.+",
    "graphql-tag",
    "events",
    "eslint-plugin-jsdoc",
    "memfs",
    "open",
    "textlint",
    "textlint-rule-no-dead-link",
    "@elasticpath/textlint-rule-no-dead-relative-link",
    "ts-generator",
    "webdriverio"
  ]
}

FuelLabs / fuels-ts