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 5 compatibility #230

Merged
merged 6 commits into from
May 19, 2024
Merged

Svelte 5 compatibility #230

merged 6 commits into from
May 19, 2024

Conversation

EMH333
Copy link
Owner

@EMH333 EMH333 commented May 11, 2024

Still a bit of work to do for full Svelte v5 support, but mostly test changes. A very good sign that a single ? is all that is needed in the actual plugin code

I'll note that in compatibility testing, this PR is able to successfully compile files with Svelte 5 (not with any new Svelte 5 syntax yet, local testing shows that to be working correctly though).

If I can get a few 🚀 reactions or comments on this PR with folks who have been able to test it successfully, I'll go ahead and release it as 0.8.1. Otherwise, to avoid the risk breaking preexisting projects, I'll probably aim for a 0.9.0 release with more sweeping changes (ie breaking type import compatibility) before 1.0.0 to coincide with Svelte 5.

Related to #204

@EMH333 EMH333 added this to the v1.0.0 milestone May 11, 2024
@EMH333 EMH333 mentioned this pull request May 17, 2024
7 tasks
@EMH333 EMH333 self-assigned this May 17, 2024
@tgf9
Copy link

tgf9 commented May 17, 2024

I ran these commands and was able to successfully build and run my existing
Svelte 4 project with the branch svelte-5-compat. 700 lines of Svelte.

git clone https://github.com/EMH333/esbuild-svelte.git
cd esbuild-svelte
git checkout svelte-5-compat
npm install
npm run build
npm link

# Verify esbuild-svelte is present
ls $(npm root -g)

cd /path/to/project
npm install --save-dev svelte@latest
npm link esbuild-svelte

# Verify linked esbuild-svelte is used
npm ls esbuild-svelte

node esbuild.js

Next I installed Svelte 5.

npm install --save-dev svelte@next
npm link esbuild-svelte

# Verify linked esbuild-svelte is used
npm ls esbuild-svelte

node esbuild.js

(Later clean up global links with npm remove esbuild-svelte -g)

... and 💥 my project blew up with a bunch of warnings like this.

▲ [WARNING] Import "SvelteComponent" will always be undefined because the file "node_modules/svelte/src/internal/index.js" has no exports [import-is-undefined]

    src/App.svelte:3:1:
      3 │   SvelteComponent,

which, I'm guessing actually means I'm using Svelte 5?!

During runtime, I get this error:

Uncaught Error: Your application, or one of its dependencies, imported from 'svelte/internal', which was a private module used by Svelte 4 components that no longer exists in Svelte 5.

My App.svelte is just <p>hello</p> and my index.js entry is just import App from "./App.svelte".

@tgf9
Copy link

tgf9 commented May 17, 2024

OK, my project must just be weird. I tried this and got Svelte 5 working correctly. 🙌

npx degit EMH333/esbuild-svelte/example-js my-svelte-app
cd my-svelte-app
npm install --force --save-dev svelte@next
npm link esbuild-svelte

# Verified with
npm ls esbuild-svelte

# Updated entry.js to use Svelte 5's mount function

npm run build

cd dist/
python -m http.server

Update:
OK, it looks like svelte-routing is the library giving me trouble.

✘ [ERROR] Could not resolve "svelte/internal/disclose-version"

    ../node_modules/svelte-routing/src/Link.svelte:1:7:
      1 │ import "svelte/internal/disclose-version";
        ╵        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  You can mark the path "svelte/internal/disclose-version" as external to exclude it from the
  bundle, which will remove this error and leave the unresolved path in the bundle.

When I removed svelte-routing from my project, everything built fine for Svelte 5.

Copy link

@Greenheart Greenheart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating and maintaining this plugin! I made some progress towards stabilizing this branch!

Testing and findings

I tested with my project and and got the same build time warnings as mentioned in the comment above (#230 (comment)). I also get the same runtime error about importing from svelte/internal.

One important difference though is that I don't use any external dependencies. I'm only compiling a single Svelte component that only has two imports, and both from Svelte itself:

import { onMount } from 'svelte'
import { writable } from 'svelte/store'

Looking at the compiled output after running esbuild with the svelte-5-compat version of this esbuild-svelte, it seems like no internal Svelte code gets added to the build. This in turn breaks the components.

This made me wonder if this is something happening within the esbuild-svelte package. So I investigated it anything looked suspicious. And then I found it:

The problem

In the devDependencies of package.json, this branch still specifies ^4.2.11.

This seems to cause the esbuild-svelte plugin to compile svelte code using the Svelte 4 compiler. And this explains the build time warnings, as well as the runtime errors, because svelte/internal is deprecated in Svelte 5.

Working solution

Update svelte to something like ^5.0.0-next.136 in devDependencies of the main package.json for this plugin. Then the svelte-5-compat branch works well for compiling svelte 5 code (but still only using the svelte 4 syntax).

@EMH333
Copy link
Owner Author

EMH333 commented May 17, 2024

Cool, thank you both for testing! I believe this issue will go away when released since the devDependencies won't be installed and this plugin will instead will rely on the peerDependency version of Svelte (whatever version that may be).

Could y'all try removing the node_modules folder from your clone of esbuild-svelte after you've built the plugin? I suspect esbuild is trying to be smart and trying to use the "closest" svelte instead of the top level project svelte version. Though maybe I should just test this myself... 😄

FWIW, I've been changing the dev dependency to Svelte 5 and it has worked without issue, I just didn't want to upstream it since Svelte 5 will be experimentally supported for 0.8.1 and not the main version. Speaking of which, I should add changelog notes...

@Greenheart
Copy link

Greenheart commented May 18, 2024

That sounds possible! I made some updates that allowed me to continue with the Svelte 5 code. Maybe it will be useful in its entirety - or there might be some commits that could be cherry-picked out when it's time for the full update :)

See #233 for reference.

@EMH333
Copy link
Owner Author

EMH333 commented May 18, 2024

Great, that will be very helpful, thanks! I'll aim for a 0.8.1 release on Sunday (sorry, I'd do it sooner but it has been a long week and I want to get this right). Then we can look at merging your additional changes to fully convert everything over to Svelte 5

@Greenheart
Copy link

No rush! :) I'm unblocked by using my patched branch for the moment.

Let me know if we should split the PR into several smaller ones to allow merging parts of it.

Otherwise if you update to Svelte 5 and release 0.8.1, then the PR diff for #233 will hopefully only include the remaining changes, which could be merged once Svelte 5 is stable.

@EMH333 EMH333 merged commit b13d4a3 into main May 19, 2024
4 checks passed
@EMH333 EMH333 deleted the svelte-5-compat branch May 19, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants