-
-
Notifications
You must be signed in to change notification settings - Fork 462
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
fix(core): make metadata
available in production
#3714
fix(core): make metadata
available in production
#3714
Conversation
🦋 Changeset detectedLatest commit: d1aa67a 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 |
…ode, ensuring consistent metadata availability (e.g., `cacheOutcome`) in both development and production environments
56ae8e5
to
2c63917
Compare
metadata
available in productionmetadata
available in production
@isy @JoviDeCroock any chance you guys could review this please? |
@redroot we remove the meta for a reason, it's meant to be for development - a better solution here might be to move it away from development metadata or for us to split up metadata in dev and not dev. EDIT: I looked deeper into the few places where this transform is used and yeah, we should remove it entirely |
Co-authored-by: Jovi De Croock <[email protected]>
It seems I can not merge myself due to a lack of permissions. Could you do it, @JoviDeCroock? |
@alpavlove That's normal yes, you are not a collab/member |
@JoviDeCroock, unfortunately, it can't be released due to an error on CI |
Thanks for getting this over the line, @JoviDeCroock! |
@alpavlove @JoviDeCroock are you sure this works? I am using the latest libs and still don't see the meta. We use |
Yes, it's working on my end. I'd recommend you ensure you're using at least version |
I am using the latest libraries. Coming from the lock file:
And all the other libs are hooked with deps As I mentioned, it is working on localhost, is not working on the devel environment, and is not deployed to prod yet so I don't know. Any ideas? Do I need to configure something specific I missed? |
@RomanFausek Are you by any chance using GraphCache instead of the document cache? |
Yes, I am using graphcache. Is that it? |
@JoviDeCroock Could you provide more details, please? I'm curious why it works in one case but not in the other. Is there any workaround to get it working? 🤔 |
Because graphCache does not set that entry as the cache is far more nuanced with partial and stale reads Edit: nvm we actually did end up implementing it but we didn't release a new version of gcache |
Fixes #3715
Summary
This change addresses a bug where the
addMetadata
function was being stripped or modified in production builds, resulting in the meta field being unavailable. The absence of meta caused inconsistent behaviour between development and production environments, particularly for TTL-based re-requests and cache outcomes.Previously, there was an attempt to add back the meta to production bundles, but perhaps the babel file was missed.
Set of Changes
Modified Babel Transformer:
Updated the Babel transformer to preserve
addMetadata
calls in production, ensuring metadata (e.g.,cacheOutcome
) is always available.Affected Package:
@urql/core: The core package was updated to fix the issue.