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

Add preprocessor sourcemap #157

Merged
merged 2 commits into from
Nov 25, 2020

Conversation

benmccann
Copy link
Member

No description provided.

Copy link
Member

@Conduitry Conduitry left a comment

Choose a reason for hiding this comment

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

This should be implemented in such a way that it does not fail with older versions of Svelte, and it doesn't look like this was done. The compiler throws an exception on unknown compiler options. If you have to bump the version of Svelte in order for existing tests to pass, then as implemented this is a breaking change.

@Conduitry
Copy link
Member

Conduitry commented Nov 25, 2020

Did the response from svelte.preprocess change in 3.30.0, or was the only change in svelte.compile? It may not have changed. The only way to do this might, unfortunately, be to check the VERSION of the copy of the compiler we're using. The other option is to update the peerDependencies and make this a major bump.

@benmccann
Copy link
Member Author

@benmccann
Copy link
Member Author

I think this should be okay because it would pass sourcemap: undefined on < 3.30. I didn't manually test that though. Do you still think this is something that I should take a closer look at?

@Conduitry
Copy link
Member

The compiler throws if an unknown key is present in the options object, even if its value is undefined.

@benmccann
Copy link
Member Author

updated. thanks for taking a look!

@benmccann benmccann merged commit 7503feb into sveltejs:master Nov 25, 2020
@milahu milahu mentioned this pull request Jan 15, 2021
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.

3 participants