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

breaking: remove legacy package.json files #8490

Closed
wants to merge 5 commits into from

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented Apr 12, 2023

Now that we only support newer versions of Node we can simply rely on exports.

This is a breaking change because it requires bundlers to be able to handle the exports field. This is the case for

  • Vite 3/4
  • Rollup 2/3 (using @rollup/plugin-node-resolve)
  • Webpack 5

If you're relying on a bundler that does not know how to deal with export maps, you need to update its version to one that supports it (like webpack 4->5) or switch to another bundler.

xxkl1 and others added 4 commits April 11, 2023 15:20
Instead of "both", which doesn't make sense at that point.
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Apr 12, 2023

@benmccann is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@benmccann benmccann changed the title remove legacy package.json files chore: remove legacy package.json files Apr 12, 2023
@dummdidumm
Copy link
Member

dummdidumm commented Apr 12, 2023

Is it safe to do this? It's not only about the node version, but also about bundlers. Do all recent versions of Rollup/Webpack/Vite and our REPL auto-detect and use the package exports correctly?

@benmccann
Copy link
Member Author

I think this way is actually more safe. Everything handles exports at this point. The package.json files take precedence over exports and as much as I've tried to fix it, Vite fails to handle it which we're currently working around with certain config.

@benmccann
Copy link
Member Author

benmccann commented Apr 12, 2023

Hmm. Actually, webpack 4 may not support it. So I guess there's a question of whether we want to drop support for webpack 4 or not.

It looks to me that about half of webpack users are still on webpack 4. And it looks like about 16% of Svelte users are on webpack: https://npmcharts.com/compare/@sveltejs/vite-plugin-svelte,rollup-plugin-svelte,svelte-loader?log=false&interval=7. So it'd probably be about 8% of users that are on webpack 4 and would need to stay on Svelte 3 until they're able to upgrade to webpack 5.

@benmccann
Copy link
Member Author

@non25 @syvb do you have thoughts on whether we could drop webpack 4 support in Svelte 4?

@non25
Copy link

non25 commented Apr 12, 2023

If they are reluctant enough to not upgrade webpack to v5 to this day, then I think they definitely won't be upgrading svelte to v4 anytime soon. 😁

I'm kinda out of the loop on what your plans are regarding svelte 4 and what it will require to refactor in user's 5-40k LoC apps. 🤷‍♂️

@benmccann
Copy link
Member Author

benmccann commented Apr 12, 2023

Thanks. There will be a few breaking changes in Svelte 4 like dropping older versions of Node, but hopefully nothing terribly bad. You can see more on https://svelte.dev/roadmap if you're interested

@syvb
Copy link

syvb commented Apr 12, 2023

Dropping Webpack 4 support sounds reasonable to me - Webpack 5 has been out for a while now, and people who are still on Webpack 4 probably won't want to update Svelte either. And updating Webpack 4 -> 5 isn't particularly difficult.

@benmccann benmccann added this to the 4.x milestone Apr 12, 2023
types: './index.d.ts'
}, null, ' '));

fs.writeFileSync(`${dir}/index.d.ts`, `export * from '../types/runtime/${dir}/index';`);
Copy link
Member

Choose a reason for hiding this comment

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

We can't get rid of this line because else TypeScript without moduleResolution: bundler won't find the type definitions anymore.

@dummdidumm dummdidumm changed the title chore: remove legacy package.json files breaking: remove legacy package.json files Apr 18, 2023
@benmccann
Copy link
Member Author

this PR is trashed. closing in favor of #8515

@benmccann benmccann closed this Apr 18, 2023
@benmccann benmccann deleted the legacy-packages branch April 18, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants