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

maybeJSX throws a fatal error instead of a warning #10123

Open
7 tasks done
benjamingwynn opened this issue Sep 14, 2022 · 7 comments
Open
7 tasks done

maybeJSX throws a fatal error instead of a warning #10123

benjamingwynn opened this issue Sep 14, 2022 · 7 comments

Comments

@benjamingwynn
Copy link

benjamingwynn commented Sep 14, 2022

Describe the bug

There should be some environmental variable to skip the maybeJSX variable bailing, or it should be a warning.

This flags incorrectly for us, as we're specifically serving Svelte files without the Svelte plugin for integration with a file editor.

This comes from line 198 of importAnalysis.ts:

const maybeJSX = !isVue && isJSRequest(importer)

Also see: #6246

Reproduction

https://stackblitz.com/edit/vitejs-vite-9nnfuu?file=counter.js

System Info

System:
    OS: macOS 12.5.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 87.70 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Firefox: 104.0.1
    Safari: 15.6.1

Used Package Manager

npm

Logs

17:34:04 [vite] Internal server error: Failed to parse source for import analysis because the content contains invalid JS syntax. If you are using JSX, make sure to name the file with the .jsx or .tsx extension.
  Plugin: vite:import-analysis
  File: /home/projects/vitejs-vite-9nnfuu/foo/bar.svelte
  1  |  <script>
  2  |    let foo = 'bar';
  3  |  </script>
     |           ^
  4  |  <h1>this is a file that should be served as is</h1>
      at formatError (file:///home/projects/vitejs-vite-9nnfuu/node_modules/vite/dist/node/chunks/dep-665b0112.js:40828:46)
      at TransformContext.error (file:///home/projects/vitejs-vite-9nnfuu/node_modules/vite/dist/node/chunks/dep-665b0112.js:40824:19)
      at TransformContext.transform (file:///home/projects/vitejs-vite-9nnfuu/node_modules/vite/dist/node/chunks/dep-665b0112.js:37495:22)
      at async Object.transform (file:///home/projects/vitejs-vite-9nnfuu/node_modules/vite/dist/node/chunks/dep-665b0112.js:41077:30)
      at async loadAndTransform (file:///home/projects/vitejs-vite-9nnfuu/node_modules/vite/dist/node/chunks/dep-665b0112.js:37338:29)

Validations

@sapphi-red
Copy link
Member

When *.svelte is served, Vite passes the file to plugins to allow plugins to transpile it. This happens even if svelte plugin is not used. (related: #9773, #3828)

For now, if you want to serve *.svelte files as-is, you could use *.svelte?raw for a workaround.

@benjamingwynn
Copy link
Author

Thanks for the reply!

For now, if you want to serve *.svelte files as-is, you could use *.svelte?raw for a workaround.

We tried this but the file seems to always serve with a wraper for ESM compatability. Is there a way to serve from Vite without the export default ... wrapper around the contents of the file?

If not, we can use the proxy option to get our backend web server to handle serving these files, but it'd be nice to do it through Vite for its various features when resolving paths and the like.

@sapphi-red
Copy link
Member

We tried this but the file seems to always serve with a wraper for ESM compatability.

I forgot about it. A bit hacky but I think this would work.

const res = await fetch('./foo.svelte?raw')
const content = await res.text()
const m = content.match(/^export default ("[^]+")$/)
const realContent = m ? /* dev */ JSON.parse(m[1]) : /* build */ content

@benjamingwynn
Copy link
Author

const realContent = m ? /* dev */ JSON.parse(m[1]) : /* build */ content

Although this would work, ideally we'd just have the server serve the file as-is, is this not possible with Vite currently?

As eluded to, we're working on a Svelte file editor, so it has a bundler on the frontend that needs to get src files as-is (including resolving *.svelte content from node_modules), we can add this realContent hack into the frontend bundler plugin, but I rather add workarounds to the server than the client worker (if possible!)

We can use the resolveId section of the plugin to add ?raw, so ideally we'd be able to do ?raw&nowrap or something similar; rather than have to hack the load section of the plugin.

I'm a bit amiss as to why this behaviour is different between dev and prod build, but maybe I'm misunderstanding your comment and you can clear that up or point me in the right direction of documentation/code to read?

Thanks again for all your help! VIte has been an incredible addition to our stack 🎉

@sapphi-red
Copy link
Member

Although this would work, ideally we'd just have the server serve the file as-is, is this not possible with Vite currently?

Yes, ideally Vite should return the raw file with fetch('foo.svelte'). But it's not currently possible.
Vite needs to pass svelte files to plugin pipeline when it's imported (e.g. import('foo.svelte')) and serve it as-is foo.svelte when it's fetched.
So Vite needs to differentiate fetch('foo.svelte') and import('foo.svelte') by HTTP header or something. But there isn't any (Sec-Fetch-Dest is not supported by Safari yet.).
For now, Vite currently assumes svelte files are imported and not fetched.

I rather add workarounds to the server than the client worker (if possible!)

I've come up with another idea. If you always need a raw file, a plugin like this might work as an workaround:

const plugin = {
  name: 'plugin',
  configureServer(server) {
    server.middleware.use(async (req, res, next) => {
      if (req.url.endsWith('.svelte')) { // needs to handle more precisely
        const content = await fs.readFile(path.resolve(root, req.url))
        res.end(content)
        return
      }
      next()
    })
  }
}

@benjamingwynn
Copy link
Author

Thank you for the workarounds and insight, the issue makes a lot of sense now!

Is there an existing Vite issue/discussion open for Sec-Fetch-Dest/?raw's different behaviour? I see you're already testing for the header in a certain case for making sure SPA's don't try to pull in the index as a script:

if (url?.endsWith('.html') && req.headers['sec-fetch-dest'] !== 'script') {

I think it would make sense to add the relevant functionality for ?raw to wrap in export default depending on Sec-Fetch-Dest even without Safari support, as this only seems to affect the development environment and you could easily warn on the console if the header was missing.

Happy to fork this off into another issue if one doesn't exist to avoid confusion with the maybeJSX bug ?

@sapphi-red
Copy link
Member

There's slightly related discussion here: #9981.

I think it would make sense to add the relevant functionality for ?raw to wrap in export default depending on Sec-Fetch-Dest even without Safari support, as this only seems to affect the development environment and you could easily warn on the console if the header was missing.

This won't work. Even if the plugin handling ?raw returned the raw content without export default, the plugin after that plugin will try to parse that content as JS and the errors will happen.

Happy to fork this off into another issue if one doesn't exist to avoid confusion with the maybeJSX bug ?

IMO this error is correct because plugin pipeline expects the content to be JS.
The problem here is that foo.svelte is passed to plugin pipeline, even if it is fetched and not imported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants