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

Drop CJS version of adapter-netlify #7401

Merged
merged 3 commits into from
Oct 27, 2022

Conversation

benmccann
Copy link
Member

The CJS version should be unused since #6666, so this should not be a user visible change. I still added it to the changelog though so that we can cut a new release

If this works well, a followup might be to see if we can get rid of the Rollup bundling step in this project since it seems the other adapters don't need it

@changeset-bot
Copy link

changeset-bot bot commented Oct 26, 2022

🦋 Changeset detected

Latest commit: 195d8cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sveltejs/adapter-netlify Patch
@sveltejs/adapter-auto Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rich-Harris
Copy link
Member

I think we'll still need the bundling step, because otherwise apps will need to declare dependencies on things like set-cookie-parser. Same applies to adapter-node with its dependencies on e.g. sirv and polka

@Rich-Harris Rich-Harris merged commit 97d45c0 into sveltejs:master Oct 27, 2022
@benmccann benmccann deleted the netlify-esm-only branch October 27, 2022 21:20
@benmccann
Copy link
Member Author

I think we'll still need the bundling step, because otherwise apps will need to declare dependencies on things like set-cookie-parser. Same applies to adapter-node with its dependencies on e.g. sirv and polka

Presumably the adapter dependencies would just get installed when you run npm run install and then the adapter would bundle them. I don't think the user would need to declare any dependencies. But anyway, not really a priority for me at the moment

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.

3 participants