Skip to content

Conversation

csvn
Copy link
Contributor

@csvn csvn commented Aug 1, 2025

This is just a potential suggestion, but I think it can be nice to just need "fresh" to import anything needed on the server. Currently, there may be multiple import statements ("fresh" + "fresh/runtime") to get simple things from Fresh, e.g.:

import { page } from "fresh";
import { asset } from "fresh/runtime";

Given that the @fresh/core package already imports from @fresh/core/runtime, I don't think this should be a regression for existing code or esbuild.

graph TD;
    fresh <--> Server;
    fresh/runtime <--> Server;
    fresh/runtime <--> Browser["Browser (Islands)"];
Loading

Though I don't know if there is a desire to avoid exports existing from multiple entrypoints, or if there is some reason JSR prevents publishing like this.

@csvn csvn force-pushed the feat/runtime-re-export branch from 7329f53 to 983e566 Compare August 1, 2025 16:31
@marvinhagemeister
Copy link
Collaborator

Having one import certainly makes it look cleaner! I'm worried a bit about how this affects user's. When you're inside an island, you're not supposed to import functionality that can never work there.

This means we need to be either super careful about only ever referencing/exporting things that can work in both Deno and the browser. Or we would need to employ a bundler plugin to rewrite all fresh imports to a different module behind the scenes. But doing that would lead to the problem that no IDE will complain about that because it's not aware of the different runtime tragets. In the IDE everything will look ok, and then when you run it, it will break. Then user's will report bugs in our tracker.

I can certainly relate to the desire to not have to think about those things and it being easier to have only one fresh import to worry about for users. I'm a bit worried about the tradeoffs that come along with that.

@csvn
Copy link
Contributor Author

csvn commented Aug 2, 2025

Having one import certainly makes it look cleaner! I'm worried a bit about how this affects user's. When you're inside an island, you're not supposed to import functionality that can never work there.

This means we need to be either super careful about only ever referencing/exporting things that can work in both Deno and the browser. Or we would need to employ a bundler plugin to rewrite all fresh imports to a different module behind the scenes. But doing that would lead to the problem that no IDE will complain about that because it's not aware of the different runtime tragets. In the IDE everything will look ok, and then when you run it, it will break. Then user's will report bugs in our tracker.

I can certainly relate to the desire to not have to think about those things and it being easier to have only one fresh import to worry about for users. I'm a bit worried about the tradeoffs that come along with that.

That makes a lot of sense. I can think of a potential addition that could help, but maybe it's not worth the effort. But we could create a lint rule in Fresh that lints for incorrectly importing anything from fresh in Islands. TBH, a user might already be doing that incorrectly, even if we don't merge imports. But it's true auto-import could exacerbate the problem, but perhaps bundling with tree-shaking would still be able to handle it if no server code is included?

I'd be happy to give it a try, and to me it makes sense if Fresh started with supplying it's own lint rules in this project (instead of bundled in Deno).

// /islands/Foo.tsx
import { HttpError } from "fresh";
                          ^^^^^^^ Importing from "fresh" is not supported in Islands and Browsers.

@csvn csvn mentioned this pull request Aug 2, 2025
2 tasks
@csvn
Copy link
Contributor Author

csvn commented Aug 2, 2025

I made #3179 as a Draft for a new base of Fresh specific lint rules. I could finish up that PR if that seems promising.

@csvn csvn force-pushed the feat/runtime-re-export branch from 983e566 to e4738b3 Compare August 5, 2025 22:21
@csvn csvn force-pushed the feat/runtime-re-export branch from e4738b3 to 8f92af4 Compare September 11, 2025 22:33
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.

2 participants