Conversation
🦋 Changeset detectedLatest commit: 826e441 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
05bed0c to
e0fdb42
Compare
e0fdb42 to
d2aec21
Compare
There was a problem hiding this comment.
For a second I thought that the red & were them being removed on the updated version as was concerned 😅
RitaDias
left a comment
There was a problem hiding this comment.
This looks good to me though I would just want to have someone else that is a bit more knowledgeable on the config side of things double check that everything is good to go :)
There was a problem hiding this comment.
Thanks, this is great. I am wondering if it's time to just do a major version bump and break things?
- There are imports to things like
@sanity/image-url/lib/browser/builderon the internet, which will break with this change - Tempting to go ESM-only, Node.js >= 20 at this point. We list node >= 10 in the package, but also use
@sanity/browserslist-configwhich saysmaintained node versions. These are not compatible with each other. Additionally, the dependencies introduced in the signed URL PR requires Node v20.19, so it would make sense to align here anyway. - Would love to use a named export instead of default export
package.json
Outdated
| "index.js", | ||
| "urlForImage.js" | ||
| "src", | ||
| "build/compat-shims.mjs" |
There was a problem hiding this comment.
Don't think we need to ship this in the published npm module?
There was a problem hiding this comment.
Ah thank you, good catch, build/compat-shims.mjs was a holdover from my first attempt at backwards compatibility (it doesn't actually exist anymore).
Would you prefer to remove src too?
There was a problem hiding this comment.
Would you prefer to remove src too?
If the sourcemaps reference src I tend to include it, but also don't feel strongly about it.
There was a problem hiding this comment.
Cool, they do, I'll include it.
|
Thanks for taking a look @rexxars and for catching the node mismatches, I glossed over that. I was trying to weigh up whether or not a major would be “worth it” without new features, but if you’re comfortable with that then it would definitely simplify things. Feels like it could be a bit of a pain to maintain full backwards compatibility, evidenced by my attempt to cover the types but not accounting for those browser imports. Would we introduce a named export and keep/soft deprecate the default export to make things slightly less painful? |
We could do that! Up to you :) |
|
I’ve added a follow up commit that makes breaking changes:
|
I think |
I very much agree. I prefer the former for terseness but the latter seems sensibly unambiguous and probably the safer option, so I've added a commit to refactor to that. |
|
Should likely add a |
|
Absolutely, I assume I also need to add a changeset before merging. I was planning on speaking to docs about this in advance of releasing too as there are a lot of references to Thanks for the review 🙇. |
f80b5ba to
826e441
Compare

To support signed URLs (the next PR upstack), I decided to add a specific signed version of the url builder available from a different export path
/signed. This was to avoid bundling the crypto dependencies we rely on for signing, as this functionality will likely only be used by a very small subset of users.I thought it would be a good time to "modernize" the repo a little to support that work. That included:
jest withvitest for testsmicrobundlewith@sanity/pkg-utilsfor building@sanity/pkg-utilsstrictestsrc/types.ts from the default export pathI’ve also added in a
postbuild/prepack step which outputs some files for backwards compatibility. This is to support users who are currently importing types from paths like@sanity/image-url/lib/types/types.