Add app-template-preact-typescript #1238
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/1221dz4vs |
| @@ -0,0 +1,43 @@ | |||
| { | |||
| "name": "@snowpack/app-template-preact-typescript", | |||
| "version": "1.10.2", | |||
There was a problem hiding this comment.
Should it be 1.0.0?
There was a problem hiding this comment.
We can make it 1.0.0, yeah. Admittedly the version numbers for the templates have gone through a bit of thrashing with our migration into the Snowpack monorepo, as Lerna bumps them for dependency changes and Snowpack changes.
| src: '/_dist_', | ||
| }, | ||
| plugins: [ | ||
| '@snowpack/plugin-dotenv', |
There was a problem hiding this comment.
Maybe be we should remove @snowpack/plugin-dotenv from all templates, because it's not actually being used?
There was a problem hiding this comment.
I think we could do a better job at communicating this, but the idea is to match parity with Create React App as much as possible. But, agreed that this may be better suited as "documenting how easy it is to add this plugin" vs. "added for everyone, by default".
Lets leave it in for this PR to match all others, but great call out.
| @@ -0,0 +1,23 @@ | |||
| { | |||
| "include": ["src", "types"], | |||
| "exclude": ["node_modules"], | |||
There was a problem hiding this comment.
I think "exclude": ["node_modules"] can be removed from all tsconfigs in all typescript templates
There was a problem hiding this comment.
Yeah good call. If you wanted to remove that as part of this PR, I’d be fine with that.
| "@babel/preset-react": "^7.10.4", | ||
| "@babel/preset-typescript": "^7.10.4", | ||
| "@prefresh/snowpack": "^2.0.1", | ||
| "@snowpack/app-scripts-preact": "^1.9.3", |
There was a problem hiding this comment.
Maybe it would be better to remove @snowpack/app-scripts-preact from dependencies and move jest.config to the template?
There was a problem hiding this comment.
I think we’re heading in that direction, but the Jest configuration is just annoying enough where I think it’s better to leave app-scripts-preact as-is for now. However, we may be able to ditch it when #1197 gets a little further! But for this PR I think this is fine for now.
|
Hey @smashercosmo! Thanks for adding this template. Our test suite failed because you need to add the Snapshot to our test suite (this ensures the template builds as expected throughout changes). To update it, run |
| declare module '*.avif' { | ||
| const ref: string; | ||
| export default ref; | ||
| } |
|
When I run |
|
@smashercosmo 71 failed tests doesn't sound right, but I can confirm that our test suite was broken on master overnight. If you rebase on master, you should now see CI passing. |
smashercosmo
force-pushed
the
app-template-preact-typescript
branch
from
5ca7850 to
c49cccb
Compare
|
Ok. Done. The problem was that I didn't build before running tests. |
Changes
Implements #1217