honojs / honox

HonoX - Hono based meta framework
https://hono.dev
MIT License
1.4k stars 38 forks source link

It would be nice to place islands anywhere, not just in routes and islands directories. #159

Closed liltechnomancer closed 4 months ago

liltechnomancer commented 4 months ago

Hi! πŸ‘‹

Firstly, thanks for your work on this project! πŸ™‚

Today I used patch-package to patch honox@0.1.15 for the project I'm working on.

I wanted to be able to place islands anywhere not just within route and islands

Here is the diff that solved my problem:

This should probably be done in a more elegant way, but I wanted to provide what worked for me. If you like the idea, I can make the commits to add config options or something to allow for this.

diff --git a/node_modules/honox/dist/vite/inject-importing-islands.js b/node_modules/honox/dist/vite/inject-importing-islands.js
index 78c7d0f..47a28aa 100644
--- a/node_modules/honox/dist/vite/inject-importing-islands.js
+++ b/node_modules/honox/dist/vite/inject-importing-islands.js
@@ -8,7 +8,7 @@ import { IMPORTING_ISLANDS_ID } from "../constants.js";
 const generate = _generate.default ?? _generate;
 async function injectImportingIslands() {
   const isIslandRegex = new RegExp(/(\/islands\/|\_[a-zA-Z0-9[-]+\.island\.[tj]sx$)/);
-  const fileRegex = new RegExp(/(routes|_renderer|_error|_404)\/.*\.[tj]sx$/);
+  const fileRegex = new RegExp(/(.|routes|_renderer|_error|_404)\/.*\.[tj]sx$/);
   const cache = {};
   const walkDependencyTree = async (baseFile, dependencyFile) => {
     const depPath = dependencyFile ? path.join(path.dirname(baseFile), dependencyFile) + ".tsx" : baseFile;
@@ -37,8 +37,10 @@ async function injectImportingIslands() {
       }
       const hasIslandsImport = (await walkDependencyTree(id)).flat().some((x) => isIslandRegex.test(normalizePath(x)));
       if (!hasIslandsImport) {
+        
         return;
       }
+
       const ast = parse(sourceCode, {
         sourceType: "module",
         plugins: ["jsx", "typescript"]
diff --git a/node_modules/honox/dist/vite/island-components.js b/node_modules/honox/dist/vite/island-components.js
index eb99b62..8998758 100644
--- a/node_modules/honox/dist/vite/island-components.js
+++ b/node_modules/honox/dist/vite/island-components.js
@@ -157,10 +157,11 @@ function getIslandComponentName(root, id, options) {
     return path.resolve(id2).startsWith(islandDirectoryPath);
   };
   const matchIslandPath = options?.isIsland ?? defaultIsIsland;
+  
   if (!matchIslandPath(id)) {    
     return null;
   }
-  const match = id.match(/(\/islands\/.+?\.tsx$)|(\/routes\/.*\_[a-zA-Z0-9[-]+\.island\.tsx$)/);
+  const match = id.match(/(\/islands\/.+?\.tsx$)|(\/app\/.*\_[a-zA-Z0-9[-]+\.island\.tsx$)/);
   if (!match) {    
     return null;
   }

This issue body was partially generated by patch-package.

yusukebe commented 4 months ago

Hi @liltechnomancer

Sorry for the delayed response. Yes. This should be fixed. If Some PR will be merged, I can work on it! Thanks.

liltechnomancer commented 4 months ago

@yusukebe No need to apologize, thanks for all the amazing work!

yusukebe commented 4 months ago

Hi @liltechnomancer

I will create the PR by referring to your code. Can I make you a co-author for the PR?

liltechnomancer commented 4 months ago

If you would like! That would be cool πŸ˜„