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

[Bug?]: undefined error on exported variable in page route #1591

Closed
2 tasks done
ryoid opened this issue Jul 29, 2024 · 5 comments
Closed
2 tasks done

[Bug?]: undefined error on exported variable in page route #1591

ryoid opened this issue Jul 29, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@ryoid
Copy link
Contributor

ryoid commented Jul 29, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

In a page route when trying to access an exported variable X is not defined error is thrown during SSR.

Interestingly the error does not occur when the var is not exported and seems to only occur in route file.

// routes/index.tsx

// Removing 'export' works
export const A = "A"

// A is not defined
const B = A

export default function Home() {
  return <div>Hello World</div>;
}

Expected behavior 🤔

Variable access should work.

Steps to reproduce 🕹

Steps:

Stackblitz started from start.solid.new

  1. Run in dev
  2. Visit the index page (SSR)
  3. Check console for error

Error:

[vite] Error when evaluating SSR module /home/projects/zzmivvvnlw.github/src/routes/index.tsx?pick=default&pick=$css:
|- ReferenceError: A is not defined
    at eval (/home/projects/zzmivvvnlw.github/src/routes/index.tsx?pick=default&pick=$css:8:14)
    at async instantiateModule (file:///home/projects/zzmivvvnlw.github/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:52699:5)

Context 🔦

Exporting config objects.

Your environment 🌎

"@solidjs/router": "^0.14.1",
    "@solidjs/start": "^1.0.6",
    "solid-js": "^1.8.18",
    "vinxi": "^0.4.1"
@ryoid ryoid added the bug Something isn't working label Jul 29, 2024
@alfredomariamilano
Copy link

I have the same issue and I think it was caused by updating to the latest vinxi version.

@ryansolid
Copy link
Member

It's caused by the fix we did from the HMR page reload. Before we weren't properly reloading HMR on pages which was causing hydration errors on new routes. The problem now is that when the routes reload it appears that we get circular deps issues in HMR when things are exported from the page files and used downstream.

All I can recommend at this point is don't export from page routes. The way code splitting and bundling work it is too easy for things to get in a weird state. Maybe we should just find a way to error/warn you in this case. But the types of loops it can cause especially if the export is used by components used in nested page sections messes with code splitting. Strongly recommend having a separate file that has things that are imported into both locations.

@alfredomariamilano
Copy link

It's caused by the fix we did from the HMR page reload. Before we weren't properly reloading HMR on pages which was causing hydration errors on new routes. The problem now is that when the routes reload it appears that we get circular deps issues in HMR when things are exported from the page files and used downstream.

All I can recommend at this point is don't export from page routes. The way code splitting and bundling work it is too easy for things to get in a weird state. Maybe we should just find a way to error/warn you in this case. But the types of loops it can cause especially if the export is used by components used in nested page sections messes with code splitting. Strongly recommend having a separate file that has things that are imported into both locations.

Yeah, I realised it was a circular dependency and fixed it, but it wasn't from a page route. I just wish the error would actually be helpful and inform of the circular dependency instead of the current incorrect error. I will look into vinxi to see if I can pinpoint it and open a PR.

@ryansolid
Copy link
Member

In general this is an interesting type of issue I'm seeing people hit more often than I thought. Because of the way code splitting works I could never recommend this pattern and never even thought of it. But people tend to want to do this and I'm not sure we can safe guard against it unless we somehow error when processing the routes. While not all situations end up in breaking code splitting it is incredibly common and basically defeats the point of file system routing.

Unfortunately I hit very similar issues when building SolidStart internals and had the same undefined problem. This silently fails at layer lower than us so I don't really have any recourse here. I'm open to suggestions, but not recommending exporting things other than default/route from page routes is all I can really put out there.

@alfredomariamilano
Copy link

In general this is an interesting type of issue I'm seeing people hit more often than I thought. Because of the way code splitting works I could never recommend this pattern and never even thought of it. But people tend to want to do this and I'm not sure we can safe guard against it unless we somehow error when processing the routes. While not all situations end up in breaking code splitting it is incredibly common and basically defeats the point of file system routing.

Unfortunately I hit very similar issues when building SolidStart internals and had the same undefined problem. This silently fails at layer lower than us so I don't really have any recourse here. I'm open to suggestions, but not recommending exporting things other than default/route from page routes is all I can really put out there.

Erroring out during the creation of routes sounds like the best (and maybe only) option. This behaviour also happens if you import a component from a different directory than routes and then export it as default from a route, so that's another thing we could catch. I think I saw the code that creates the routes and it already checks for default AND named exports, so we could just throw there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants