Skip to content

fix: Handle multiple exports from the same file in fs-router#1936

Merged
atilafassina merged 1 commit intosolidjs:mainfrom
dennev:fix/File-System-Routing
Sep 10, 2025
Merged

fix: Handle multiple exports from the same file in fs-router#1936
atilafassina merged 1 commit intosolidjs:mainfrom
dennev:fix/File-System-Routing

Conversation

@dennev
Copy link
Copy Markdown
Contributor

@dennev dennev commented Aug 6, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

What is the current behavior?

If a default function in the file routes folder references an export function in the same file, a not defined error occurs and cannot be rendered.

What is the new behavior?

The hydration process works even when the default function and its multiple referenced export functions are in the same file.

Other information

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Aug 6, 2025

🦋 Changeset detected

Latest commit: 98f6c5f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solidjs/start Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link
Copy Markdown

netlify bot commented Aug 6, 2025

Deploy Preview for solid-start-landing-page ready!

Name Link
🔨 Latest commit 98f6c5f
🔍 Latest deploy log https://app.netlify.com/projects/solid-start-landing-page/deploys/68c19bac9c421c0008bcaa36
😎 Deploy Preview https://deploy-preview-1936--solid-start-landing-page.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Aug 7, 2025

@brenelz
Copy link
Copy Markdown
Contributor

brenelz commented Aug 19, 2025

This solves the 3 open issues? That would be great. Ill take a bit of a look here

@dennev dennev force-pushed the fix/File-System-Routing branch from 404c0ac to acdd3bb Compare August 19, 2025 04:49
@dennev dennev requested a review from brenelz August 19, 2025 06:03
@Brendonovich
Copy link
Copy Markdown
Collaborator

This is a decent solution, though after devinxi I might look at adding an omit filter as well as pick, so our router config could be omit: ["route"], pick: ["$css"]. That way the module ID wouldn't grow as more exports are added.

@brenelz
Copy link
Copy Markdown
Contributor

brenelz commented Aug 28, 2025

@Brendonovich so we should merge this in?

@dennev dennev force-pushed the fix/File-System-Routing branch from 10e5ba4 to 062b609 Compare August 31, 2025 08:51
test: add a test route for multiple exports handling and correct variable typos

chore: add changeset for handling multiple exports in fs-router and tests
@atilafassina atilafassina force-pushed the fix/File-System-Routing branch from 062b609 to 98f6c5f Compare September 10, 2025 15:39
Copy link
Copy Markdown
Member

@atilafassina atilafassina left a comment

Choose a reason for hiding this comment

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

Amazing work! 🏆

@atilafassina atilafassina merged commit 12f3a3d into solidjs:main Sep 10, 2025
9 checks passed
@dennev dennev deleted the fix/File-System-Routing branch September 11, 2025 02:08
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.

[Bug?]: Component is not defined

4 participants