fix: File watcher isn't initialized on start-up #398

going-confetti · 2025-01-09T12:57:11Z

Description

Fix for the regression of the file watcher not being initialized and not working until you refresh the app
Fix for the file watcher that prevents not supported files from getting into the file trees
Groundwork for the test data support

How to Test

Check that fs-related stuff works as expected
- Adding, deleting, renaming files
- Existing links between generators and recordings
- External scripts etc.

Fix for file watcher:

While having the app open, open the scripts directory in Finder/Explorer
Paste a non-js file (e.g. an image)
Make sure it doesn't show up in the file tree

Groundwork for data files support:

Restart the app
Verify that the Data folder has been created in the studio workspace folder
Open Sidebar.tsx
Replace line 20 with const { recordings, generators, scripts, data } = useFiles(searchTerm)
Add a new FileTree for data files:

<FileTree
  label="Test data"
  files={data}
  noFilesMessage="No scripts found"
/>

Open the data folder in Finder/Explorer and paste a csv/json file into it
Verify that the file shows up in the file tree
Restart the app to make sure not only watcher, but also the getFiles handler works as expected

Checklist

I have performed a self-review of my code.
I have added tests for my changes.
I have run linter locally (npm run lint) and all checks pass.
I have run tests locally (npm test) and all tests pass.
I have commented on my code, particularly in hard-to-understand areas.

going-confetti

Self-review

going-confetti · 2025-01-09T15:04:42Z

src/components/Layout/Sidebar/Sidebar.hooks.ts

 }

 function useFolderContent() {
  const recordings = useStudioUIStore((s) => orderByFileName(s.recordings))
  const generators = useStudioUIStore((s) => orderByFileName(s.generators))
  const scripts = useStudioUIStore((s) => orderByFileName(s.scripts))
+  const data = useStudioUIStore((s) => orderByFileName(s.data))


Perhaps data is too vague?

I'm leaning towards a yes, maybe we should be explicit and call it testData ?

going-confetti · 2025-01-09T15:05:54Z

src/main.ts

+    const file = getStudioFileFromPath(filePath)
+
+    if (!file) {
+      return
+    }


Previously, you could add an arbitrary file to one of the watched folders and ui:add-file would still be fired

going-confetti · 2025-01-09T15:07:35Z

src/store/ui/useStudioUIStore.ts

        }
      }),
-    removeFile: (path) =>
+    removeFile: (file) =>
      set((state) => {


It now seems to me that it'd better to store all files in a flat map and get recordings, generators, scripts`, etc. using a zustand selector

going-confetti · 2025-01-09T15:08:30Z

src/utils/file.ts

-  return fileName.endsWith('.har')
-}
-
-export function isGenerator(fileName: string) {


With the addition of test data, this is no longer accurate. Instead, we get StudioFile objects from the main process, so we can always check the type directly

going-confetti · 2025-01-09T15:09:17Z

src/views/Generator/Generator.utils.ts

-export const scriptExists = async (fileName: string) => {
-  return window.studio.ui
-    .getFiles()
-    .then((files) => files.scripts.includes(fileName))
-}


This is not needed, because we always have the current list of files in the ui store

going-confetti · 2025-01-09T17:01:57Z

package.json

@@ -82,7 +82,7 @@
    "@tanstack/react-query": "^5.55.4",
    "@vitejs/plugin-react": "^4.3.1",
    "allotment": "^1.20.2",
-    "chokidar": "^3.6.0",
+    "chokidar": "^4.0.3",


Sep 2024 update: v4 is out! It decreases dependency count from 13 to 1, removes support for globs, adds support for ESM / Common.js modules, and bumps minimum node.js version from v8 to v14. Check out upgrading.

Llandy3d

LGTM!

Llandy3d · 2025-01-09T17:45:05Z

src/components/Layout/Sidebar/Sidebar.hooks.ts

 }

 function useFolderContent() {
  const recordings = useStudioUIStore((s) => orderByFileName(s.recordings))
  const generators = useStudioUIStore((s) => orderByFileName(s.generators))
  const scripts = useStudioUIStore((s) => orderByFileName(s.scripts))
+  const data = useStudioUIStore((s) => orderByFileName(s.data))


I'm leaning towards a yes, maybe we should be explicit and call it testData ?

cristianoventura

LGTM! I agree with renaming instances of data to test data.

src/components/Layout/Sidebar/Sidebar.tsx

cristianoventura · 2025-01-09T18:58:05Z

src/constants/workspace.ts

@@ -5,3 +5,4 @@ export const PROJECT_PATH = path.join(app.getPath('documents'), 'k6-studio')
 export const RECORDINGS_PATH = path.join(PROJECT_PATH, 'Recordings')
 export const GENERATORS_PATH = path.join(PROJECT_PATH, 'Generators')
 export const SCRIPTS_PATH = path.join(PROJECT_PATH, 'Scripts')
+export const DATA_PATH = path.join(PROJECT_PATH, 'Data')


Perhaps the directory name should also be Test Data?

going-confetti · 2025-01-10T09:10:21Z

@Llandy3d @cristianoventura re: data/test data name - I've been meaning to ask product about this, will update the PR after I get an answer

going-confetti force-pushed the chore/improve-file-watch branch from 8b26276 to 0a67a00 Compare January 9, 2025 14:50

going-confetti commented Jan 9, 2025

View reviewed changes

going-confetti marked this pull request as ready for review January 9, 2025 15:16

going-confetti requested a review from a team as a code owner January 9, 2025 15:16

going-confetti requested review from cristianoventura and Llandy3d January 9, 2025 15:16

going-confetti commented Jan 9, 2025

View reviewed changes

Llandy3d previously approved these changes Jan 9, 2025

View reviewed changes

cristianoventura previously approved these changes Jan 9, 2025

View reviewed changes

going-confetti added 6 commits January 10, 2025 10:16

chore: Improve file watcher

fd82823

fix: fix loading

49cb50d

wip: remove test data from sidebar

266f722

fix: additional filtering

1dffe6f

fix: fix file handlers

bd62f5b

remove test data tree

75dcb0f

going-confetti dismissed stale reviews from cristianoventura and Llandy3d via 75dcb0f January 10, 2025 09:17

going-confetti force-pushed the chore/improve-file-watch branch from 7d70e55 to 75dcb0f Compare January 10, 2025 09:17

fix: file watcher initialization

4b150bb

going-confetti changed the title ~~chore: Improve file watcher~~ fix: File watcher isn't initialized on start-up Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: File watcher isn't initialized on start-up #398

fix: File watcher isn't initialized on start-up #398

going-confetti commented Jan 9, 2025 •

edited

Loading

going-confetti left a comment

going-confetti Jan 9, 2025

Llandy3d Jan 9, 2025

going-confetti Jan 9, 2025

going-confetti Jan 9, 2025

going-confetti Jan 9, 2025

going-confetti Jan 9, 2025

going-confetti Jan 9, 2025

Llandy3d left a comment

Llandy3d Jan 9, 2025

cristianoventura left a comment

cristianoventura Jan 9, 2025

going-confetti commented Jan 10, 2025

fix: File watcher isn't initialized on start-up #398

Are you sure you want to change the base?

fix: File watcher isn't initialized on start-up #398

Conversation

going-confetti commented Jan 9, 2025 • edited Loading

Description

How to Test

Checklist

going-confetti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Llandy3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristianoventura left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

going-confetti commented Jan 10, 2025

going-confetti commented Jan 9, 2025 •

edited

Loading