good-fences icon indicating copy to clipboard operation
good-fences copied to clipboard

Allow for tags to be applicable to only certain files

Open ahlec opened this issue 3 years ago • 10 comments

Description

Currently, tags are universally applicable to all files in the tree of the fence.json file. This works well for smaller projects or for repositories that make use of directories, but can be limiting for flatter directory hierarchies. Since imports and exports are powered off of tags, it can be problematic when you want to permit the importing of one or two select files, without allowing the entire directory.

For example:

src/
  application-1/
    fence.json     <— Good with importing anything from shared

  components/
    fence.json     <— Wants to allow shared/theme.ts, but would require also allowing shared/redux

  shared/
    redux/
      ...

    theme.ts

For this, I took the mentality of specifying that certain dependencies were only allowed for specific files, and added the same logic to tags. By default, all string tags operate like they did — globally applicable to the entire tree of the config file. But you can now also specify them as an object, providing a glob path/paths to allow for tagging only specific files:

{
  "tags": [
      "shared",
      {
        "applicableTo": "theme.ts",
        "tag": "theme"
      }
  ]
}

Doing this, components/ could now just import shared/theme.ts — it will still be an error to import shared/redux.

Testing

I've updated the unit tests and wrote some new ones concerning tag resolution and how getTagsForFile works. I've also updated the sample application to showcase an example of this.

Areas of Concern

Primarily, I'd want to make sure that my logic for applying the glob pattern relative to the directory the config file was defined in makes sense. Programmatically it runs, but it might make some assumptions that I'd like to make sure are actually valid.

Also, let me know if there are any stylistic or formatting issues.

ahlec avatar Feb 10 '21 22:02 ahlec

@ahlec — I'm curious, is it that difficult to move the files you want to share (theme.js, in this case) into its own directory? My aim is a simple mental model where tags correspond to directories. I'm having a hard time imagining a case where that isn't adequate, unless for some reason you really can't move code into a subdirectory.

smikula avatar Feb 11 '21 20:02 smikula

theme.ts could be moved to its own directory at shared/theme/theme, but overall that would lead to a lot of one-item directories. Files that were already their smallest possible version (ie, there's not enough to spread across multiple files in a directory) would be alone in that directory and you'd be forced either to double-specify the name (once for the directory, once for the file) or you'll have a ton of index files.

I think it also helps with building tighter fences as well. For instance, many of the codebases I work on have unit test files alongside their React components. When I'm building the fence files, I need to permit all files in the hierarchy to be allowed to import enzyme or jest, even though really I only want to allow the unit test files to import that. With something like this, I'd be able to set up a fence like this (in theory? I haven't tested it yet tbh):

{
  "tags": [
    "components",
    {
      "applicableTo": "**/*.unit.tsx",
      "tag": "unit-tests"
    }
  ],
  "dependencies": [
    "react",
    {
      "accessibleTo": "unit-tests",
      "dependency": "enzyme"
    }
  ]
}

ahlec avatar Feb 11 '21 20:02 ahlec

theme.ts could be moved to its own directory at shared/theme/theme, but overall that would lead to a lot of one-item directories. Files that were already their smallest possible version (ie, there's not enough to spread across multiple files in a directory) would be alone in that directory and you'd be forced either to double-specify the name (once for the directory, once for the file) or you'll have a ton of index files.

I think it also helps with building tighter fences as well. For instance, many of the codebases I work on have unit test files alongside their React components. When I'm building the fence files, I need to permit all files in the hierarchy to be allowed to import enzyme or jest, even though really I only want to allow the unit test files to import that. With something like this, I'd be able to set up a fence like this (in theory? I haven't tested it yet tbh):

That makes sense. It doesn't hurt to have the option for those who want to use it; I just might not promote it as the the typical approach.

I'm pretty busy with other stuff at the moment, but I'll keep this PR around and try to get it in when I have a chance.

smikula avatar Feb 11 '21 20:02 smikula

Thank you, and no rush!! And certainly, if you'd like this implemented a different way, broken apart, or even if you don't want to take it, that's fine; I don't want to shoehorn something in if it breaks how you view the tool overall. This just felt to me like something I kept wanting to reach for in some advanced cases, and felt like a natural pairing for the advanced dependencies control. But I'm super happy to work with you if you want to iterate on this some more.

ahlec avatar Feb 11 '21 21:02 ahlec

Hey! I'd wanted to check in if there was any progress on this front/if you'd had time to think about this proposal?

ahlec avatar May 25 '21 20:05 ahlec

@ahlec Sorry, yes, my intent is to take this; I just want to take a close look at it before I merge it and it's hard for me to carve out time for that.

smikula avatar May 26 '21 18:05 smikula

Wanted to touch base again on this PR since I've run in to some more use cases for this the other day.

ahlec avatar Aug 27 '21 20:08 ahlec

Is there anything I'd be able to do to help move this along?

ahlec avatar Oct 23 '21 03:10 ahlec

Is there anything I'd be able to do to help move this along?

No, sorry, I've been letting it linger. It's tough to prioritize since I'm swamped enough with my day-job priorities. But it's back on my radar, so I'll see if I can squeeze it in.

smikula avatar Oct 25 '21 22:10 smikula

Is someone who would like finish and merge this PR? I would like to have this exact feature in my app. I have this file structure.

features/
   featureName/
      actionA/
         handler.ts
         handler.test.ts
      actionB/
        handler.ts
        handler.test.ts
tests/
    generic.ts
    ...

Files with .test extension import files from ./tests. What I want to achieve is disallowing to import from ./tests for files without .test extension.

czlowiek488 avatar May 28 '22 20:05 czlowiek488