-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add minimal template #1090
Add minimal template #1090
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). astro-www – ./www🔍 Inspect: https://vercel.com/pikapkg/astro-www/C4ankSjrxs63czGiN4SXXtx3PZXL [Deployment for a5d39ea canceled] astro-docs – ./docs🔍 Inspect: https://vercel.com/pikapkg/astro-docs/HzL6KnB7RqYerJReKUqRPXKZUmVf [Deployment for a5d39ea canceled] |
examples/minimal/astro.config.js
Outdated
@@ -0,0 +1,3 @@ | |||
export default { | |||
renderers: [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For another PR, but I really want to make this the default soon, so this file wouldn't be necessary either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be wrong, but an empty array might trigger their installation anyway. Therefore, with your approval, I’d like to remove this file as well, and move my +1 over to your suggestion that "no renderers" be the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it's about time to do that, but not in the scope of this PR. Let's leave this file for now.
A todo item for me is to add some warnings about the upcoming behavior change to the next version so we can make the switch in the version after that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an empty array doesn’t currently limit the renderers, should I still keep this line? Not sure if I’m to:
- A. Keep the line. It has an effect.
- B. Remove the line. It has no effect.
- C. Remove the file, because the line has no effect.
If the line has an effect, then I would vote A, but if it has no effect then I’m on the fence between B and C.
Thanks for the thoughtful dialog, @natemoo-re! Thanks for making me smile, @jasikpark! Following your suggestion, I’ve removed |
Adjacent to this PR, it'd be nice to have a setup that works similar to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! @natemoo-re do you have any more feedback or can this be merged?
@jasikpark LGTM! Feel free to merge. |
Changes
This adds a new
minimal
template tocreate-astro
.src/pages/index.astro
.astro.config.js
&snowpack.config.js
configurations to (hopefully) disable any default plugins or renderers."type": "module"
. (because it’s available in the oldest Node LTS at the time of this writing).Testing
I was unable to test this. I encountered a strange error running
npm run test:templates
:Docs
This might not have any affect on documentation.