Fastify Fruits Example - Githubissues

johntom commented 1 year ago

Hi, I'm testing your Enchance docs using your fruits example I modified app/api/fruits.mjs but no enpont gives results export async function get (req) { let fruits = ['apple','banana','pears'] return { json: { fruits }

} }

http://localhost:3000/ works http://localhost:3000/about works http://localhost:3000/fruits shows a blank page http://localhost:3000/api/fruits Oops, something went wrong /api/fruits not found

Curious to know what I'm doing wrong. I have some rich examples using fastify and htmx and evaluating this repo for comparison TIA John node 16.15.0 on windows 10 home

brianleroux commented 1 year ago

hey John, sounds like a bug possibly? localhost:3000/fruits should show the json values. can you send a link or zip up the failing code for me to look at? I can't reproduce with this level of detail

cschuller commented 1 year ago

@johntom @brianleroux

This looks like a bug on windows operating systems. It can be reproduced without an api code.

How to reproduce

Given

File ./app/pages/ping.mjs

export default function ping({html}) {
    return html`pong`;
}

File ./app/elements.mjs

import ping from './pages/ping.mjs'

const elements = {
    'page-ping': ping
};

export default elements;

When

http://localhost:8200/ping

Then

The result html contains an invalid custom element <page-\ping>

<html lang="en"><head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <title></title>
  <link rel="stylesheet" href="/_static/styles.css">
  <link rel="icon" href="/_static/favicon.svg">
</head>
<body>
    <page-\ping></page-\ping>
</body>
</html>

Analysis

I think the problem is based on a OS path separator (/ vs \).

Personal Note

I am new to enhance, but I really like the concept. I am missing some contributor guidelines.

What repo is the main repo to create general issues? design topos? feature requests?
An overview (graph) how all the repos/building blocks are connected would be very helpful.

cschuller commented 1 year ago

@brianleroux

Code Analysis

The problem is in line 24 & 25 of this code: https://github.com/enhance-dev/arc-plugin-enhance/blob/e313440cc6d0ccb36a03c6a428e6e3b067e4f8f3/src/http/any-catchall/_get-page-name.mjs#L24-L25

Explanation

For example on windows

let base = path.join(basePath, 'pages')   // C:\my-project\app\pages

// Let templatePath be: C:\my-project\app\pages\ping.mjs
let raw = templatePath
    .replace(base, '')  // \ping.mjs
    .replace(/\.mjs/g, '') // \ping
    .replace('/', '') // \ping   <-- Here we have the magic "\"
    .replace(/\//g, '-') // \ping

How to fix

Avoid hardcoded forward slash ('/') replacement when handling filepaths.

Use path.sep (https://nodejs.org/api/path.html#pathsep)

let raw = templatePath
    .replace(base, '')  // \ping.mjs
    .replace(/\.mjs/g, '') // \ping
    .replace(path.sep, '') // ping   
    .replaceAll(path.sep, '-') // ping

johntom commented 1 year ago

Hi Brian, Thanks ! John

Node 16.15.0 Edition Windows 10 Home Version 21H2 Installed on ‎4/‎9/‎2021 OS build 19044.1889 \

On Mon, Sep 12, 2022 at 1:26 PM Brian LeRoux @.***> wrote:

hey John, sounds like a bug possibly? localhost:3000/fruits should show the json values. can you send a link or zip up the failing code for me to look at? I can't reproduce with this level of detail

— Reply to this email directly, view it on GitHub https://github.com/enhance-dev/fastify-plugin/issues/1#issuecomment-1244064589, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOKLACJTLSFRHQT2VXU6W3V55RV5ANCNFSM6AAAAAAQJBAOWI . You are receiving this because you authored the thread.Message ID: @.***>

enhance-dev / fastify-plugin

Fastify Fruits Example #1