feat(server): add safety checks during dev

[mbhrznr]

this is the second attempt to land #1489, as the initial version has been reverted due to unwanted side effects.

closes #475. closes #1558. closes #1678.

supersedes #539. supersedes #1414.

this pr introduces the following check when being in dev mode:

• [x] only one _app.tsx is allowed per application and it must have a default export
• [x] only one 500.tsx and it must have a default export
• [x] only one 404.tsx and it must have a default export
• [x] ~~pascal case or kebab case islands (island name)~~ (this one became obsolete w/ 1.3)
• [x] routes must have exported handler or component
• [x] route patterns must be unique
• [x] route patterns w/ dynamic params must not have conflicts w/ other routes
• [x] if static override is used, ensure that there is no normal static directory
• [x] static file conflicts with routes (because static files have higher priority)
• [x] plugin doesn't call render/renderAsync
• [x] each defined module in a plugin must be a js/ts module
• [x] plugins should be present in the build snapshot

the functionality can be tested by changing the fixture task in deno.json to:

"fixture": "deno run -A --watch=static/,routes/ tests/fixture_dev_checks/dev.ts",

this time, the pr also contains unit tests for each individual check to (hopefully 🤞 ) prevent further mishaps.

[deer]

You should go into each fixture and start up the dev server to confirm this isn't causing issues. Perhaps we could even add a test to assert that all the fixtures adhere to these checks.

[mbhrznr]

You should go into each fixture and start up the dev server to confirm this isn't causing issues. Perhaps we could even add a test to assert that all the fixtures adhere to these checks.

will do! currently i'm writing unit tests for each of the checks to hopefully cover all of the cases 🤞 will keep the pr in draft for the time being, yet wanted to bring it up asap again for transparency reasons.

i'd also incorporate the feedback and remarks from the first pr.

[deer]

I see some changes of as -> satisfies. I guess this makes sense, but I wonder why just these three places? It seems like there should be a larger audit of the codebase to make this switch where appropriate. Therefore I think you should revert these from this PR, and then log an issue or open a separate PR if you're willing to take on the work yourself.

Some of the names in the error messages don't seem quite right:

See: _app
See: test
See: /custom.txt

I would expect:

See: routes/_app.tsx
See: routes/test.tsx
See: static/custom.txt

[mbhrznr]

as always, thank you for the feedback!

I see some changes of as -> satisfies. I guess this makes sense, but I wonder why just these three places? It seems like there should be a larger audit of the codebase to make this switch where appropriate. Therefore I think you should revert these from this PR, and then log an issue or open a separate PR if you're willing to take on the work yourself.

sorry about that! there wasn't an immediate need to do so, i just stumbled upon this while i was testing each of the available fixtures. will revert these once i push the final changes.

Some of the names in the error messages don't seem quite right:

See: _app
See: test
See: /custom.txt

I would expect:

See: routes/_app.tsx
See: routes/test.tsx
See: static/custom.txt

you're absolutely right!
this has also been brought up in the initial pr and is something i'm currently working on. this would be the final task before i'd comfortably consider this pr as "ready for review".

[mbhrznr]

improved the warnings output as suggested. also checked all of the fixtures to have some certainty that the dev checks don't introduce any unwanted behavior.

[deer]

Seems like this should eventually cover https://github.com/denoland/fresh/issues/1678#issuecomment-1683034081 as well.

[mbhrznr]

Seems like this should eventually cover https://github.com/denoland/fresh/issues/1678#issuecomment-1683034081 as well.

found a way to naively check the existence plugins in the current BuildSnapshotJson.
thanks for the hint!

[deer]

https://github.com/denoland/fresh/issues/1950#issuecomment-1768959095 is another example of a good check to add.

[deer]

An addendum to https://github.com/denoland/fresh/issues/1558: we also need to think about conflicting islands as well. And if https://github.com/denoland/fresh/pull/2253 goes through, then also static files.