Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: sync new files from WebContainer to editor #394

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

Merged
AriPerkkio merged 11 commits into stackblitz:main from isaacplmann:add-new-files-in-paths
Nov 5, 2024

Conversation

@isaacplmann
Copy link
Contributor

@isaacplmann isaacplmann commented Oct 25, 2024
edited
Loading

I work at Nx and in order to use tutorialkit for our tutorials, we need the editor to show files that are created by terminal commands.

This PR shows a prototype that enables the editor to show new files in the webcontainer that are created in a specific set of paths. This is needed to use tutorialkit for Nx tutorials.

Updates the watch lesson property to take either a boolean or string[]. If passing a string[] it is interpreted as a list of globs.

I added e2e tests for the new functionality as well.

If watch is false or undefined, existing files are not synced and new files are not created.
If watch is true, existing files are synced, but new files are not created.
If watch is an array of glob strings, existing files are synced and new files whose paths match one of the globs listed in the watch array are created in the editor. Files that are deleted in one of the glob patterns are removed from the editor. New files that are not in those paths do not get added to the editor.

Nemikolh reacted with hooray emoji Nemikolh reacted with eyes emoji
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #334 it was discussed whether we should allow fs.watch: [<glob>, <glob>]. I think instead of adding new addNewFilesInPaths, we should instead make this a default functionality of fs.watch. And add pattern matching for fs.watch to avoid performance issues.

@Nemikolh has better understanding of fs.watch so let's wait for his comments before any changes.

Copy link
Contributor Author

That API would work for me. I'm happy to implement it once @Nemikolh has a chance to review.

Copy link
Member

Hey! Thanks a ton for the PR 😃

The proposal of having watch accepting [<glob>] to restrict what get reflected in the file tree, sounds really good! 🙌

In terms of performance concerns, we won't be able to provide that down to fs.watch just yet. It's something we want to add at some point though but it should not block this PR from landing.

Copy link
Member

@Nemikolh Nemikolh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good!

Once this is implemented using watch, we can merge it ✨

Copy link
Contributor Author

PR and description updated to allow an array in the filesystem.watch property

Nemikolh reacted with hooray emoji

Copy link
Contributor Author

  • Now accounting for rename file deletion
  • Folder creation logic moved into scheduleReadFor
  • Use picomatch/posix
  • e2e tests for everything
Nemikolh reacted with hooray emoji

Copy link
Member

@Nemikolh Nemikolh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work on this @isaacplmann ! 🤩

Love it 😍

Co-authored-by: Jòan <Nemikolh@users.noreply.github.com>
Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup the code looks good to me. I'll do some manual testing before next release. Probably tomorrow.

Copy link
Member

AriPerkkio commented Oct 31, 2024
edited
Loading

/pkg-pr-new

⚡️ Your npm packages are published.
@tutorialkit/react: npm i https://pkg.pr.new/@tutorialkit/react@e288f4e
@tutorialkit/astro: npm i https://pkg.pr.new/@tutorialkit/astro@e288f4e
@tutorialkit/runtime: npm i https://pkg.pr.new/@tutorialkit/runtime@e288f4e
@tutorialkit/theme: npm i https://pkg.pr.new/@tutorialkit/theme@e288f4e
@tutorialkit/types: npm i https://pkg.pr.new/@tutorialkit/types@e288f4e

github-actions[bot] reacted with thumbs up emoji

Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bug in new file/folder creation. Empty folders are added/duplicated. Is this just related to the test case or an actual bug?

tk-fs-bug.mov

Copy link
Contributor Author

Updated documentation as well

Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @isaacplmann! 💯

@AriPerkkio AriPerkkio merged commit 3beda90 into stackblitz:main Nov 5, 2024
Copy link
Member

Released in 1.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nemikolh Nemikolh Nemikolh approved these changes

@AriPerkkio AriPerkkio AriPerkkio approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /