Skip to content
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

svelte-kit package errors due to not removing lang="ts" #2450

Closed
probablykasper opened this issue Sep 17, 2021 · 5 comments · Fixed by #2486
Closed

svelte-kit package errors due to not removing lang="ts" #2450

probablykasper opened this issue Sep 17, 2021 · 5 comments · Fixed by #2486
Labels
pkg:svelte-package Issues related to svelte-package

Comments

@probablykasper
Copy link

probablykasper commented Sep 17, 2021

Describe the bug

When running svelte-kit package, code is preprocessed but the lang="ts" attributes are not removed. This is giving me errors when trying to use the generated components. I wanted to open this issue in this repo to make sure it's either resolved before SvelteKit 1.0, or it's mentioned in the docs that preprocessing is broken.

Same issue in svelte-preprocess: sveltejs/svelte-preprocess#260
Related SvelteKit issue: #1843

Reproduction

  1. npm init svelte@next my-app with TypeScript enabled
  2. cd my-app
  3. npm install sass svelte2tsx && npm install
  4. Add src/lib/Example.svelte:
    <script lang="ts">
      const x: number = 8
    </script>
    
    <p>{x}</p>
    
    <style lang="sass">
      p
        color: red
    </style>
  5. svelte-kit package

Logs

Error from using a component generated by svelte-kit package:

5:22:33 PM [vite] Internal server error: Expected newline.
  ╷
2 │ .date-time-picker {
  │                   ^
  ╵
  ../svelte-date-picker/package/DatePicker.svelte 2:19  root stylesheet
  Plugin: vite-plugin-svelte
  File: /Users/kasper/dev/git/svelte-date-picker/package/DatePicker.svelte
  1  |  <script lang="ts">
  2  |    import { getMonthLength, getWeeks } from './utils/date-utils'
     |                     ^
  3  |    export let value = new Date()
  4  |    export let visible = true
      at Object._newRenderError (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:2203:19)
      at Object._wrapException (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:2023:16)
      at _render_closure1.call$2 (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:82163:21)
      at _RootZone.runBinary$3$3 (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:27566:18)
      at _FutureListener.handleError$1 (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:26096:21)
      at _Future__propagateToListeners_handleError.call$0 (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:26395:49)
      at Object._Future__propagateToListeners (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:12483:77)
      at _Future._completeError$2 (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:26241:9)
      at _AsyncAwaitCompleter.completeError$2 (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:25910:12)
      at Object._asyncRethrow (/Users/kasper/dev/git/kryp/node_modules/sass/sass.dart.js:12286:17)

System Info

System:
    OS: macOS 10.15.7
    CPU: (8) x64 Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz
    Memory: 677.92 MB / 32.00 GB
    Shell: 3.2.2 - /usr/local/bin/fish
  Binaries:
    Node: 14.16.0 - /var/folders/44/3sxbyc2d6wg90mj1s7s59bnm0000gn/T/fnm_multishells/19056_1631295056028/bin/node
    Yarn: 1.22.10 - /var/folders/44/3sxbyc2d6wg90mj1s7s59bnm0000gn/T/fnm_multishells/19056_1631295056028/bin/yarn
    npm: 6.14.11 - /var/folders/44/3sxbyc2d6wg90mj1s7s59bnm0000gn/T/fnm_multishells/19056_1631295056028/bin/npm
  Browsers:
    Brave Browser: 93.1.29.77
    Chrome: 93.0.4577.82
    Firefox: 89.0.2
    Safari: 14.1.2

Severity

serious, but I can work around it

@dummdidumm dummdidumm added the pkg:svelte-package Issues related to svelte-package label Sep 17, 2021
@bluwy
Copy link
Member

bluwy commented Sep 19, 2021

Even though this is actually a real issue, I don't think the fix should be in SvelteKit nor svelte-preprocess. There are 3rd party preprocessor that needs a simple way to remove the lang as well, so it should be supported by Svelte instead, which is tracked here: sveltejs/svelte#5900

@dummdidumm
Copy link
Member

I tend to add a quick fix for this into SvelteKit until we have a way forward. I'm not sure if Svelte core should be responsible of this, or if it should be the preprocessors. If the preprocessor rework RFC is implemented in some form, it means that every preprocessor has the whole source content at its hand and then should be responsible (IMO) for removing the lang tag.

@bluwy
Copy link
Member

bluwy commented Sep 19, 2021

I tend to add a quick fix for this into SvelteKit until we have a way forward. I'm not sure if Svelte core should be responsible of this, or if it should be the preprocessors. If the preprocessor rework RFC is implemented in some form, it means that every preprocessor has the whole source content at its hand and then should be responsible (IMO) for removing the lang tag.

A quick fix would be nice in the meantime since the svelte fix (sveltejs/svelte#6611) doesn't seem it would be merged soon. Though maybe we can extract the attributes feature out and leave the custom parser aside?

I do think Svelte core should be responsible for it though since any script/style preprocessor outside of svelte-preprocess would need a common way of removing attributes, and also to prevent the need to manually sequence the preprocessors. The RFC would definitely be a welcome change once the API has been agreed upon.

https://github.com/sveltejs/svelte-preprocess/blob/main/docs/preprocessing.md#replace

Interesting, we could use that for svelte-kit package perhaps.

@probablykasper
Copy link
Author

Even though this is actually a real issue, I don't think the fix should be in SvelteKit nor svelte-preprocess. There are 3rd party preprocessor that needs a simple way to remove the lang as well, so it should be supported by Svelte instead, which is tracked here: sveltejs/svelte#5900

Absolutely, main reason for opening this issue is just so something is done about it before 1.0

dummdidumm pushed a commit that referenced this issue Sep 24, 2021
Fixes #2450 by doing a regex-based string replacement. This should be handled elsewhere in the long run.
dummdidumm added a commit that referenced this issue Sep 24, 2021
Fixes #2450 by doing a regex-based string replacement. This should be handled elsewhere in the long run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:svelte-package Issues related to svelte-package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants