-
Notifications
You must be signed in to change notification settings - Fork 921
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
Fix importing from Svelte files #764
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/e8lzotxuk |
beaf392
to
d48cf3e
Compare
d48cf3e
to
1fba5d4
Compare
packages/snowpack/src/util.ts
Outdated
@@ -30,6 +30,8 @@ const LOCKFILE_HASH_FILE = '.hash'; | |||
export const HAS_CDN_HASH_REGEX = /\-[a-zA-Z0-9]{16,}/; | |||
// NOTE(fks): Must match empty script elements to work properly. | |||
export const HTML_JS_REGEX = /(<script.*?type="?module"?.*?>)(.*?)<\/script>/gms; | |||
export const SVELTE_VUE_REGEX = /(<script[^>]+)(.*?)<\/script>/gms; |
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’m thinking it’s safer to keep existing behavior for HTML—only allow <script type="module">
—but make an exception for Vue & Svelte where all <script>
tags are parsed as JS. Is this correct?
1fba5d4
to
a7af94b
Compare
test/build/build.test.js
Outdated
@@ -54,13 +54,14 @@ describe('snowpack build', () => { | |||
`File not found in snapshot: ${entry.path2.replace(actual, '')}/${entry.name2}`, | |||
); | |||
|
|||
// don’t compare web_modules contents for Windows | |||
if (process.platform === 'win32' && entry.path1.includes('web_modules')) continue; |
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.
This commit was brought to you by
- const file = "src/App.svelte";
+ const file = "src\\App.svelte";
9527fdf
to
9e5dacf
Compare
2b2baef
to
586f2cf
Compare
It still does not work in the 2.11.1, I am referring to #752 for reproducing steps, the error is, however:
|
Sorry, I can't reproduce. Did you try running with |
Changes
Fixes #745, where
import
statements weren’t detected within Svelte files. The issue was that build was just fine (this PR adds a new test anyway, even though it was actually passing already without the fix), but this was an issue in the dev server. It turned out it was an issue with the installer, where it wouldn’t auto-detect imports.We were scanning all
<script type="module">
tags (HTML), but not<script>
tags (Svelte, Vue). This PR changes that.Screenshots
Testing
In the new
test/build/svelte-import
test, runyarn snowpack dev
and ensure the dev server loads.