TanStack / router

๐Ÿค– Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
8.29k stars 664 forks source link

Route files replaced with "Hello /route" boilerplate #2151

Open timoxley opened 3 months ago

timoxley commented 3 months ago

Using vite with @tanstack/router-plugin/vite. Route files with real content are getting clobbered with boilerplate. i.e.

import { createFileRoute } from '@tanstack/react-router'

export const Route = createFileRoute('/myroute')({
  component: () => <div>Hello /myroute!</div>
})

This has popped up a number of times, but doesn't happen consistently. Normally I just revert the file in my IDE.

But this time, at some point between testing some changes and committing the file it did the clobbering and I managed to lose all the changes and actually committed the boilerplate to source control. (yes i should have checked the diff)

Only just noticed a day later when I tried to demo the new route so I look like a doofus and the original content is not recoverable. ๐Ÿ˜ฟ๐Ÿ˜ฟ๐Ÿ˜ฟ

Platform

    "@tanstack/react-router": "^1.48.1",
    "@tanstack/router-cli": "^1.47.0",
    "@tanstack/router-devtools": "^1.48.1",
    "@tanstack/router-plugin": "^1.47.0",

I haven't tested with latest version but I did only update a few days ago.

My guess as to what's happening

Maybe git or the ide does something funny with the file which triggers the plugin into thinking the file is empty (and it might have been, briefly) and async operations between that tool and the route boilerplater get a bit tangled.

 // on file changed...
 if (await isFileEmpty()) { // it's empty (at this point)
    // but file becomes no longer empty between these two operations
    await writeBoilerplate()
 }
sequenceDiagram

    participant G as Git (or IDE)
    participant F as File
    participant T as Tan

    G ->> F: "Truncate File"
    activate G
    F -->> T: "Report File Changed!"
    activate T

    T ->> F: "Is File Empty?"
    Note right of T: if (await IsFileEmpty()) {<br />...  
    F -->> T: "File is Empty"
    Note right of G: Meanwhile...
    G ->> F: "Write New Content"
    deactivate G
    Note right of F: File is no longer empty...

    Note right of T: await WriteBoilerplate()
    T ->> F: "Write Boilerplate"
    deactivate T
    Note right of F: Clobbered New Content!
SeanCassiere commented 3 months ago

I haven't experienced this for a while now. That being said, we've done as much as we can to stop this from happening whilst the dev server is running.

These are the checks we've added have so far:

Checking that the routeCode is empty. https://github.com/TanStack/router/blob/4d787a8dfc4233d6e9d0cc850aad05af4f58fff3/packages/router-generator/src/generator.ts#L311-L319

Only updating the written values for routes instead of pasting the full template. https://github.com/TanStack/router/blob/4d787a8dfc4233d6e9d0cc850aad05af4f58fff3/packages/router-generator/src/generator.ts#L341-L357

Running a block when the generator is running to not double-run it. https://github.com/TanStack/router/blob/4d787a8dfc4233d6e9d0cc850aad05af4f58fff3/packages/router-plugin/src/router-generator.ts#L29-L44

TruDan commented 2 months ago

Happening to us too. Funnily enough, also when we are trying to demo our app to our boss! ๐Ÿ˜‚ Workaround being to rollback using either Git, or webstorm's Local History

We use a mix of Linux + Windows environments, also using git (with frequent updates on both ends) and Webstorm 2024.1.4

Node version: 20.11.0

    "@tanstack/react-form": "^0.23.3",
    "@tanstack/react-query": "^5.45.1",
    "@tanstack/react-router": "^1.38.1",
    "@tanstack/react-virtual": "^3.7.0",

My insights on the issue (albiet not as detailed as OP)

It seems like some sort of race condition between ((git || webstorm) && tanstack) where both are trying to write to the file at the same time. where (git || webstorm) empties the file completely before writing perhaps so tanstack see's it as empty and therefore writes its boilerplate.

I have witnessed once or twice the 'reset' files actually being a mash-up of the boilerplate + the real code (before reset). But most of the time when it does happen its just like the OP described.

for example:

import {createFileRoute} from '@tanstack/react-router'
import {PageHeader} from "@/components/PageHeader.tsx";
import {useSuspenseQuery} from "@tanstack/react-query";
import {Fab, List, MenuItem, Paper} from "@mui/material";
import {getProjects} from "@/api/queryOptions.ts";
import {HeroLink} from "@/components/HeroLink.tsx";
import {FilterList} from "@mui/icons-material";
import {useAppLayout} from "@/hooks/useAppLayout.tsx";

export const Route = createFileRoute('/_authenticated/projects')({
    component: Projects
})

function Projects() {
    const {data: projects} = useSuspenseQuery(getProjects());

    const {bottomNavigationHeight} = useAppLayout();

    return (
        <>
            <PageHeader title="Projects" />

            <Paper>
                <List>
                    {projects.map(p => (<MenuItem key={p.id} component={HeroLink} to={'/projects/$projectId'} params={{projectId: p.id}}>{p.projectName}</MenuItem>))}
import { createFileRoute } from '@tanstack/react-router'

export const Route = createFileRoute('/_authenticated/projects')({
  component: () => <div>Hello /_authenticated/projects!</div>
})

or the tanstack stuff at the start and then half of the real code after it - this makes me believe both are writing at the same time (for the record i'm the one on Windows)

Another thing that may or may not be relevant, since both members of our team use different OS's, our line endings are different of course and each time a change is made to a route page we see that the routeTree.gen.ts updates but only line endings, shouldn't be a problem, but perhaps this causes some updates to files or something? or perhaps if some route pages are LF and others CRLF the checking for a route's existence doesnt work correctly?

Does the generator use file-locks to encase the read + update of the file?

cyphase commented 2 weeks ago

Some drive-by, half-baked ideas for this issue:

  1. Could TanStack Router (TSR) use file moves instead of writing directly to the file?
  2. TSR could read routeTree.gen.ts in order to to see what files already exist, and only populate an empty route file if it's not already in routeTree.gen.ts.
  3. Add a (configurable?) delay before populating an empty file to make sure it stays empty.
  4. Allow disabling auto-population of empty route files.
SeanCassiere commented 2 weeks ago

@TruDan we are of the same mind. There's definitely a race condition somewhere where the previous content is being written into an empty file whilst the generator is also in the midst of a write. I don't belive we use any OS-level locks during writes, rather just an internal one. It's a bit out of my wheelhouse and we would happily accept any contributions here.

@cyphase to answer your points.

  1. As a plugin, we only receive file change events for "create" | "update" | "delete". As such, we aren't aware of a move. Rather we are only made aware of the following events in order, "delete" -> "create" -> "update". I believe we are doing work on the "create" step which in turn is disrupting the incoming "update" event with our own file-write for the default boilerplate.
  2. The generator works by crawling your routes directory, which checks the route files and either writes the boilerplate or updates the path in createFIleRoute|createLazyFileRoute. Then it internally builds a graph of the routes which is then translated into the output routeTree.gen.ts file. As such, we wouldn't be able to use the routeTree.gen.ts file, since it is more prone to being wrong since it has a very likely chance of being out-of-sync with what's actually on the user's filesystem.
  3. This seems like it could be a good hack. Possibly something like a 250ms at the very start which blocks all write operations. Would you be interested in contributing a PR here?
  4. The route write operation handles the updating of the path into the createFileRoute|createLazyFileRoute functions, which is needed for the type-inference and the updating of the route path (and IDs) to work. In my opinion, disabling the write brings in more harm to the user than what's brought forward with the original bug.