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

[][m]: Implement hot-reloading #620

Merged
merged 3 commits into from
Feb 2, 2024
Merged

[][m]: Implement hot-reloading #620

merged 3 commits into from
Feb 2, 2024

Conversation

mohamedsalem401
Copy link
Contributor

For #587

  • It watches for file changes in the content folder.
  • Using next-remote-watch

Copy link

vercel bot commented Jan 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
flowershow-ssr-r2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 2, 2024 1:14am

Copy link
Member

@olayway olayway left a comment

Choose a reason for hiding this comment

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

This is a great start and I would definitely use this package as well. But we also need a way to re-index changed files (to get current metadata) and index new ones (so that they can be rendered at all). Fortunately, this package allows for running custom scripts on change/add/delete/... events with --script flag! And the cool thing is that the script will be passed the event type as an argument, so in the future we could use it to only index/re-index/delete single files. For now, let's just re-index the whole folder on any type of the event. Then you can try implementing it the right way.

@mohamedsalem401
Copy link
Contributor Author

mohamedsalem401 commented Jan 26, 2024

This is a great start and I would definitely use this package as well. But we also need a way to re-index changed files (to get current metadata) and index new ones (so that they can be rendered at all). Fortunately, this package allows for running custom scripts on change/add/delete/... events with --script flag! And the cool thing is that the script will be passed the event type as an argument, so in the future we could use it to only index/re-index/delete single files. For now, let's just re-index the whole folder on any type of the event. Then you can try implementing it the right way.

An issue with the library is preventing me from using the --command flag

error - unhandledRejection: TypeError [ERR_INVALID_ARG_TYPE]: The "file" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:399:5)
    at validateString (node:internal/validators:163:11)
    at normalizeSpawnArguments (node:child_process:545:3)
    at spawn (node:child_process:750:13)
    at FSWatcher.<anonymous> (C:\Datopian\flowershow\node_modules\next-remote-watch\bin\next-remote-watch:64:13)
    at FSWatcher.emit (node:events:512:28)
    at FSWatcher.emitWithAll (C:\Datopian\flowershow\node_modules\chokidar\index.js:540:8)
    at FSWatcher._emit (C:\Datopian\flowershow\node_modules\chokidar\index.js:632:8)
    at listener (C:\Datopian\flowershow\node_modules\chokidar\lib\nodefs-handler.js:370:20) {
  code: 'ERR_INVALID_ARG_TYPE'
}

I also cannot use --script flag to run ./scripts/prebuild.mjs

Error [ERR_REQUIRE_ESM]: require() of ES Module C:\Datopian\flowershow\scripts\prebuild.mjs not supported.
Instead change the require of C:\Datopian\flowershow\scripts\prebuild.mjs to a dynamic import() which is available in all CommonJS modules.
    at FSWatcher.<anonymous> (C:\Datopian\flowershow\node_modules\next-remote-watch\bin\next-remote-watch:87:35)
    at FSWatcher.emit (node:events:512:28)
    at FSWatcher.emitWithAll (C:\Datopian\flowershow\node_modules\chokidar\index.js:540:8)
    at FSWatcher._emit (C:\Datopian\flowershow\node_modules\chokidar\index.js:632:8)
    at listener (C:\Datopian\flowershow\node_modules\chokidar\lib\nodefs-handler.js:370:20) {
  code: 'ERR_REQUIRE_ESM'
}

@olayway
Copy link
Member

olayway commented Jan 30, 2024

@mohamedsalem401 Just please make sure the E2E tests are passing and we can merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants