#475 Add warnings to terminal for possible route conflicts. #539
Conversation
There was a problem hiding this comment.
Made some feedback. Note that I'm not the maintainer, so take those with a pinch of salt
Edit: I just saw your questions, I'm going to reply at the best of my limited knowledge:
Why is there no .gitignore ?
There is none as Fresh doesn't generate bundled code and Deno doesn't install dependencies in a node_modules. And I guess no files needed to be ignored.
Are there any plans or style guide / design system for the docs ?
There is (cf. #498)
Is there a contributors guide that I'm missing?
Not yet, one is in the work (cf. #465)
Why is my prettier working differently ? It has formatted a few exisiting things differently? Dont we need a prettier config file in the root of Fresh?
Deno comes with a custom deno fmt (similar to prettier, but with a different default, for more info). VSCode should pick it up automatically, but you might need to disable your prettier in this workspace.
| console.log( | ||
| `%cCheck ${errorsList} for potential routing issues. | ||
| You may have dynamic and static routes overwriting each other. | ||
| Please check the documentation for more information. http://localhost:8000/docs/getting-started/dynamic-routes`, |
There was a problem hiding this comment.
You are pointing to your localhost:8000 here, replace it with the Fresh website.
There was a problem hiding this comment.
But when they are developing it will always be running locally here in the terminal?
There was a problem hiding this comment.
Yes, but this is a link to the documentation, not on your local project
src/dev/mod.ts
Outdated
| `%cCheck ${errorsList} for potential routing issues. | ||
| You may have dynamic and static routes overwriting each other. | ||
| Please check the documentation for more information. http://localhost:8000/docs/getting-started/dynamic-routes`, | ||
| "color: red; font-weight: bold" |
There was a problem hiding this comment.
This is more of a personal opinion, but I don't think this message should display in red, as it's more a warning. Might need confirmation from the project maintenainer though
|
@xstevenyung - Thank you so much for taking the time to answer my questions. I will fix this up some more. Makes a lot of sense about custom formatter etc, it just didnt cross my mind. |
| console.log( | ||
| `%cCheck ${errorsList} for potential routing issues. | ||
| You may have dynamic and static routes overwriting each other. | ||
| Please check the documentation for more information. http://localhost:8000/docs/getting-started/dynamic-routes`, |
There was a problem hiding this comment.
Yes, but this is a link to the documentation, not on your local project
Hi All.
I'm new to Deno and Fresh so please go easy on me ! This is my first attempt and contributing to this project. As it is quite new I was not able to find a comprehensive guide to contributing (I could help with that too if there are plans?) so I might have got quite a bit wrong. For example I've not written any tests.
This is my first attempt at solving issue/475 and I might have completely missed the point completely.
I'm looking for feedback and help on how to improve what I've done.
What it does do
routeWarningsindev/mod.tsit looks at all the routes from the manifestconsole.logsin red the following message from the list we've built up.I realise some things will need changing and improving (like I've used
anyas a type on line 153)But I was hoping for some feedback in terms of if I'm in the right direction etc?
Also I have these questions
Thank you all feedback welcomed.