fix(start): fixed route scripts filtering

[3fuyang]

The filtering before is like:

// ...
select: (state) => {
  // Assume here:
  // `[[undefined], undefined]`
  const scriptsOfEachMatch = state.matches.map((match) => match.scripts)
  // Filtered:
  // `[[undefined]]`
  const truthyScriptsOfEachMatch = scriptsOfEachMatch.filter(Boolean)
  // Turns out to be:
  // `[undefined]`
  // which will cause `TypeError` when `undefined` gets destructured below
  const truthyScripts = truthyScriptsOfEachMatch.flat(1)

  return {
    scripts: truthyScripts.map(({ children, ...script }) => ({ // <- TypeError: Cannot destructure property 'children' of 'undefined' as it is undefined.
      // ...
    })),
  }
},
// ...

Which will cause error when specifying scripts like:

export const Route = createRootRoute({
  head: () => ({
    // ...
    scripts: [undefined], // Passes type check but will panic at runtime
  }),
  // ...
})

This PR moves the filter(Boolean) after flattening. But I'm not sure if the following patterns are equivalent:

arr.filter(Boolean).flat(1).filter(Boolean) // Semantically correct

arr.flat(1).filter(Boolean) // Less code

[schiller-manuel]

we need an e2e test for this

[3fuyang]

Hi @schiller-manuel, it seems the existing e2e are all related to examples, so I'm not sure where to put the e2e case for this. But I found the start package already includes unit tests for scripts, so I added it there instead. PTAL

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 4fc30eb.

Command	Status	Duration	Result
nx affected --targets=test:eslint,test:unit,tes...	✅ Succeeded	5m 6s	View ↗
nx run-many --target=build --exclude=examples/*...	✅ Succeeded	42s	View ↗

---

☁️ Nx Cloud last updated this comment at 2024-12-28 20:21:19 UTC

[schiller-manuel]

I would put an e2e test in this project: https://github.com/TanStack/router/tree/main/e2e/start/basic

add new routes for this setup and then access them via the playwright test and add some assertions

[pkg-pr-new]

Open in Stackblitz

More templates

• tanstack-router-react-example-authenticated-routes
• tanstack-router-react-example-basic
• tanstack-router-react-example-basic-default-search-params
• tanstack-router-react-example-basic-file-based
• tanstack-router-react-example-basic-file-based-codesplitting
• tanstack-router-react-example-react-query
• tanstack-router-react-example-basic-react-query-file-based
• tanstack-router-react-example-basic-ssr-file-based
• tanstack-router-react-example-basic-ssr-streaming-file-based
• tanstack-router-react-example-basic-virtual-file-based
• tanstack-router-react-example-basic-virtual-inside-file-based
• tanstack-router-react-example-deferred-data
• tanstack-router-react-example-kitchen-sink
• tanstack-router-react-example-kitchen-sink-file-based
• tanstack-router-react-example-kitchen-sink-react-query
• tanstack-router-react-example-kitchen-sink-react-query-file-based
• tanstack-router-react-example-large-file-based
• tanstack-router-react-example-location-masking
• tanstack-router-react-example-navigation-blocking
• tanstack-router-react-example-quickstart
• tanstack-router-react-example-quickstart-esbuild-file-based
• tanstack-router-react-example-quickstart-file-based
• tanstack-router-react-example-quickstart-rspack-file-based
• tanstack-router-react-example-quickstart-webpack-file-based
• router-monorepo-react-query
• router-mono-simple
• tanstack-router-react-example-scroll-restoration
• tanstack-search-validator-adapters
• tanstack-start-example-basic
• tanstack-start-example-basic-react-query
• tanstack-start-example-basic-auth
• tanstack-start-example-basic-rsc
• tanstack-start-example-clerk-basic
• tanstack-start-example-counter
• tanstack-start-example-convex-trellaux
• tanstack-start-example-large
• tanstack-start-example-supabase-basic
• tanstack-router-react-example-with-framer-motion
• tanstack-start-example-trellaux
• tanstack-router-react-example-with-trpc
• tanstack-router-react-example-with-trpc-react-query

@tanstack/arktype-adapter

npm i https://pkg.pr.new/@tanstack/arktype-adapter@3082

@tanstack/create-router

npm i https://pkg.pr.new/@tanstack/create-router@3082

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/@tanstack/eslint-plugin-router@3082

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@3082

@tanstack/react-cross-context

npm i https://pkg.pr.new/@tanstack/react-cross-context@3082

@tanstack/create-start

npm i https://pkg.pr.new/@tanstack/create-start@3082

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@3082

@tanstack/react-router-with-query

npm i https://pkg.pr.new/@tanstack/react-router-with-query@3082

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@3082

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@3082

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@3082

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@3082

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@3082

@tanstack/start

npm i https://pkg.pr.new/@tanstack/start@3082

@tanstack/start-vite-plugin

npm i https://pkg.pr.new/@tanstack/start-vite-plugin@3082

@tanstack/valibot-adapter

npm i https://pkg.pr.new/@tanstack/valibot-adapter@3082

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/@tanstack/virtual-file-routes@3082

@tanstack/zod-adapter

npm i https://pkg.pr.new/@tanstack/zod-adapter@3082

commit: 4fc30eb

[3fuyang]

Hi @schiller-manuel , thanks for the guidance!

I've added a new /scripts route and the corresponding case to the start/basic example. Here are a few tips for review:

• Two empty script files script.js and script2.js are added to public for testing.
• I have to include the added script files to TSConfig.include so that it does not conflict with the global ESLint configuration for typed linting.
• I found that a case was marked with only, thus only this case gets evaluated when testing. I removed the only declaration and please let me know if this was intentional.
• The invoking a server function with custom response status code case fails when running e2e tests, so I modified the assertion to make it pass.

BTW please let me know whether I should remove the UT added to the start package.

[3fuyang]

Oops, the test seems to behave differently between headless and UI mode, ill check it

Update: Replaced the assertion with toHaveCount() to avoid inconsistency between headless and UI mode.

[schiller-manuel]

very nice, thanks a lot!