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

Suggestions: Ability to clear and re-initialise, or expose interals #13

Closed
coxmi opened this issue Oct 31, 2023 · 15 comments · Fixed by #18
Closed

Suggestions: Ability to clear and re-initialise, or expose interals #13

coxmi opened this issue Oct 31, 2023 · 15 comments · Fixed by #18
Assignees
Labels
enhancement New feature or request

Comments

@coxmi
Copy link

coxmi commented Oct 31, 2023

Hello,

Thanks for node-file-router!

I'm trying to get this working in watch mode while developing. Ideally after a route file has been edited/rebuilt, the next request should reflect the updates, rather than the cached module in node.

I've tried re-running the initFileRouter function after an update and passing the new instance into the app middleware, which doesn't work:

let router = {}
async function setupRouter() {
    router.handler = initFileRouter({ baseDir: ... })
}

async function serve() {
    const app = express()
    await setupRouter()
    app.use(async (req, res) => router.handler(req, res))
    app.listen(port)
}

// run the server
await serve()

...once the files have been rebuilt...

// try and override the handler
await setupRouter()

The node module cache needs to be cleared first, with:

delete require.cache[pathToModule]

Which allows it to load the most recent version.

With all that said, is there a possibility to either:

  • Include a clearCache or refresh function which clears the node module cache for any previously imported modules, and re-imports them?
  • Expose an internal function that effectively only does the routing part, e.g. (url: string): string => filePathToModule?

Thanks again :)

@Danilqa
Copy link
Owner

Danilqa commented Nov 1, 2023

@coxmi

Hello! Thank you so much for your attention and for the suggestion :)
This definitely needs to be done. I'll fix it today and will let you know when I release the new version. Or, in the worst case, till the end of the week.

@Danilqa Danilqa self-assigned this Nov 1, 2023
@Danilqa
Copy link
Owner

Danilqa commented Nov 1, 2023

@coxmi Could you please clarify which version of Node.js you're currently using? Also, when you refer to 'watch mode', are you talking about the default '--watch' option or external tools like nodemon or tsnd? It will help me to reproduce the issue.

@coxmi
Copy link
Author

coxmi commented Nov 1, 2023

Hi Dan, thank you!

I'm using esbuild's watch mode at the moment. I was hoping that I would be able to manually refresh the handler after each build, so i'm doing something like this currently:

import esbuild from 'esbuild'
import glob from 'fast-glob'

let server

const routes = glob.sync('./routes/**.{ts,js}')

const ctx = await esbuild.context({
    entryPoints: [
        'server.ts',
        ...routes
    ],
    outdir: '.build',
    bundle: true,
    platform: 'node',
    plugins: [
        { 
            name: 'onwatchend', 
            setup: build => {
                build.onEnd(async result => {
                    if (server) {
                        await server.setupRouter()
                    } else {
                        server = await import('./.build/server.cjs')
                    }
                })
            }
        }
    ]
})
    
await Promise.all([
    ctx.watch(), server.serve()
])

I can get a slimmed down example repo over to you if that would be easier?

I'm not 100% certain this is a good way to use node-file-router with a compilation step, but I'm scratching my head at how I might otherwise implement compilation, and have it update the server on each change when developing.

Thanks again :)

@coxmi
Copy link
Author

coxmi commented Nov 1, 2023

I've created a test repo here:
https://github.com/coxmi/node-file-router-test

(npm run dev to start in esbuild's watch mode)

@Danilqa
Copy link
Owner

Danilqa commented Nov 1, 2023

@coxmi wow, thank you! I've already sat down to figure it out

@coxmi
Copy link
Author

coxmi commented Nov 1, 2023

@coxmi wow, thank you! I've already sat down to figure it out

Let me know if I can help in any way, I'd be very happy to work on a pull request if there's a rough plan for the API.

@Danilqa
Copy link
Owner

Danilqa commented Nov 1, 2023

@coxmi I've implemented the watch mode in a slightly different way: coxmi/node-file-router-test#1
When a service is restarted, it works.

Does this solve your case? Or would you prefer to keep the server state without restarting it completely and just re-import the updated files?

@coxmi
Copy link
Author

coxmi commented Nov 1, 2023

Hi Dan, thanks!

That does work, but I'm running another service in the same express instance (with a long boot time), so I can't just start/restart the thread, annoyingly.

Is it feasible to expose the FileRouteResolver class instead?

Then I may be able to get it working with the SSR APIs of various bundlers too (e.g Vite's ssrLoadModulelink)

@Danilqa
Copy link
Owner

Danilqa commented Nov 1, 2023

@coxmi Alright, sure! I need to determine how the API might look. Besides import/require cache invalidation, are there any other use cases you have?

@coxmi
Copy link
Author

coxmi commented Nov 1, 2023

Amazing, thank you so much!
No other use cases at the moment, but I'll let you know if I run into anything along the way 🙂

@Danilqa
Copy link
Owner

Danilqa commented Nov 2, 2023

@coxmi Hi! Would this approach work for you? (it's just api design, not released)

 const useFileRouter = await initFileRouter({
    clearImportCache: true,
  });

This will clear the cache each time before importing. You'll simply need to reinitialize the file router. I think this is the simplest method to accomplish it without altering the existing API.

@Danilqa
Copy link
Owner

Danilqa commented Nov 7, 2023

@coxmi Hi! :) Is it still relevant for you? Could the approach above help you?

@coxmi
Copy link
Author

coxmi commented Nov 7, 2023

Hello! and sorry for the delay in getting back to you. Yes, that would be fantastic :)

@Danilqa Danilqa linked a pull request Nov 11, 2023 that will close this issue
@Danilqa
Copy link
Owner

Danilqa commented Nov 11, 2023

@coxmi Hi! I released a new version with this feature :) To see how to use it please take a look at a new section on the website. But it's actually the same as I described. Please let me know if everything is okay or if there are any areas that need improvement.

I've check in your test repo, it works.

@coxmi
Copy link
Author

coxmi commented Nov 13, 2023

Thanks so much for this!

@coxmi coxmi closed this as completed Nov 13, 2023
@Danilqa Danilqa added the enhancement New feature or request label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants