Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace nsfw with @parcel/watcher #14194

Merged
merged 6 commits into from
Oct 8, 2024
Merged

Replace nsfw with @parcel/watcher #14194

merged 6 commits into from
Oct 8, 2024

Conversation

msujew
Copy link
Member

@msujew msujew commented Sep 18, 2024

What it does

Continuation of #12784

Closes #14015

Replaces our nsfw dependency with @parcel/watcher to align with vscode.

How to test

Test file watching related capabilities and confirm that there are no regressions:

  • Editing files on the file system should be reflected in the editor
  • Editing settings.json should be reflected in changes in the preferences
  • Editing launch.json should be reflected in the debug view

Review checklist

Reminder for reviewers

@msujew msujew added filesystem issues related to the filesystem file-watchers issues related to filesystem watchers - nsfw labels Sep 18, 2024
@msujew
Copy link
Member Author

msujew commented Sep 18, 2024

@tsmaeder I'll be on vacation for a week starting Friday. In case you want to change anything in the PR, please feel free :)

@tsmaeder
Copy link
Contributor

There is a basic scenario that seems to be broken with the new watcher:

  1. Open a workspace with the theia source
  2. Do yarn to populate the node_modules folder
  3. Expand some the folders inside the node_modules folder
  4. In the Windows explorer, select all folders inside the node_modules folder
  5. Press delete
  6. Observe: Windows moves the folders to the trash
  7. Observe: the changes inside the node_modules folder are not picked up inside Theia file explorer

In master, the deletion of the modules is reflected in the Theia file navigator

@tsmaeder
Copy link
Contributor

Now this is interesting: it seems that **/node_modules/** ist part of the default excludes from file watching. So it seems as if there was a bug in the old nsfw watcher that did not exclude these files. T.b.h, I'm not sure why we're excluding the files in node_modules since we're showing them in the file explorer.

@safisa
Copy link
Contributor

safisa commented Sep 24, 2024

Hi,
I think this PR may fix the issue I have raised before: #12898
Is it right?

@msujew
Copy link
Member Author

msujew commented Sep 30, 2024

@safisa It might fix the issue, yes.

@msujew
Copy link
Member Author

msujew commented Oct 2, 2024

@planger Could you have a look at the failing playwright tests? The errors are not reproducible on Windows and the tests don't run for me on Ubuntu (Gitpod) for some reason.

@tsmaeder
Copy link
Contributor

tsmaeder commented Oct 2, 2024

I've looked into the failures of the Playwright tests. They only fail on Linux. IMO, this is a bug in the "compressed tree" and is probably unrelated to the change at hand. So, with the resources folder from the playwright test, you can do the following scenario:

  1. delete "nestedFolder2"
  2. Observe: there is no change visible and a console.log in the rendering of the compressed rows is not hit.
  3. Select the row in question
  4. Observe: nestedFolder2 disappears from the row.

The same events seem to be sent on Linux and Windows, so my suspicion is that this is a reordering of async blocks du to the different timings on the respective platforms.

@planger
Copy link
Contributor

planger commented Oct 2, 2024

Thanks for looking into this @tsmaeder! This makes sense, looking at the logs briefly.

We can take a look at the failing test in more detail, but I also suspect it might be an actual UI issue on Linux. To avoid having failing tests, I would suggest skipping this failing test for now and open a new issue.

@msujew msujew merged commit 92f4720 into master Oct 8, 2024
11 checks passed
@msujew msujew deleted the msujew/use-parcel-watcher branch October 8, 2024 08:29
@github-actions github-actions bot added this to the 1.55.0 milestone Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
file-watchers issues related to filesystem watchers - nsfw filesystem issues related to the filesystem
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Drop nsfw support
4 participants