FredKSchott / create-snowpack-app

The all-in-one app template for Snowpack. [moved]
https://www.snowpack.dev
Other
727 stars 96 forks source link

Add template snapshots #161

Closed drwpow closed 4 years ago

drwpow commented 4 years ago

We recently had some issues with our Snowpack templates, so we want to have a more robust test suite.

Before

After

Suggestion

If we added dependabot to this repo, which bumped PRs on Snowpack releases, then we’d have really darn good coverage as the snowpack dependency is the only thing we’re not able to test as part of this.

drwpow commented 4 years ago

Looks like Windows is being Windows again.

Preact/React:

- "export default "\\_dist_\\logo.png";"
+ "export default "/_dist_/logo.png";"

Svelte:

- /* src\App.svelte generated by Svelte v3.23.2 */
+ /* src/App.svelte generated by Svelte v3.23.2 */

🤔 I don’t want to hide this, as I do want to fix this. But I’m not sure if there’s an easy fix for it in this PR that adds these tests.

FredKSchott commented 4 years ago

That Preact issue looks bad though, wouldn't that be broken on Windows?

Yea, I'm not sure what the right move is here. We've used strip-comments in the past, possibly even for this exact reason. I'd be alright with stripping comments from what we compare against, since a broken / separator is fine in a comment but actually broken in the code itself.

drwpow commented 4 years ago

That Preact issue looks bad though, wouldn't that be broken on Windows?

I think so. I actually think it has to do with the import rewriting. Not unique to React or Preact; it’s in Snowpack. It’s just appeared here because I don’t think we have a test for it in Snowpack. Fixing there would be better.

FredKSchott commented 4 years ago

Hmm, that's surprising that that's not covered by our current test suite.

I'd vote to fix now in Snowpack, or skip this test until we can fix. No need to work around a bug here :)

drwpow commented 4 years ago

Merging! This doesn’t affect the templates themselves; this just strengthens this repo with a better test suite, as well as a better test environment for Snowpack itself (now you can run npm run build && cp -r pkg/* ~/path/to/create-snowpack-app locally, run yarn test in this repo, and you’ll be able to test Snowpack build pretty easily)