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: undefined is no longer a valid value for paths.relative #11185

Merged
merged 12 commits into from
Dec 10, 2023

Conversation

benmccann
Copy link
Member

closes #11073

Copy link

changeset-bot bot commented Dec 3, 2023

🦋 Changeset detected

Latest commit: 118b9f1

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Major

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

@dummdidumm
Copy link
Member

Can you create a sibling PR that logs a deprecation warning for this? Then we can merge that one into master, and then this PR can remove that warning in favor of the error

@dummdidumm dummdidumm marked this pull request as draft December 4, 2023 09:25
@benmccann
Copy link
Member Author

I don't think we want to log a deprecation warning in SvelteKit 1.0 for this one. It would trigger when the user has not set paths.relative forcing them to set it. Then when they upgrade they'd unset the option. Whereas if they just upgrade without doing anything to that option value that's exactly what we want.

I did just update the docs for this to include what the behavior in 1.0 was though. I think that we'll want to call out differences inline since there are so few differences.

@dummdidumm
Copy link
Member

Mhm that's true, nevermind then 👍

Copy link
Member

@Rich-Harris Rich-Harris left a comment

Choose a reason for hiding this comment

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

small comments on the inline documentation but otherwise LGTM

benmccann and others added 2 commits December 9, 2023 21:00
Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
@benmccann
Copy link
Member Author

I got the tests passing, but I'm quite unsure as to whether my changes to the tests are valid. Please take a look at the changes I made to the test @Rich-Harris and make sure I'm not doing something terribly wrong

@benmccann benmccann merged commit 61626de into version-2 Dec 10, 2023
13 checks passed
@benmccann benmccann deleted the rm-relative-undef branch December 10, 2023 19:19
@github-actions github-actions bot mentioned this pull request Dec 14, 2023
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