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

Check Asset Graph for source asset when dependency retargeting #9391

Draft
wants to merge 2 commits into
base: v2
Choose a base branch
from

Conversation

imbrian
Copy link
Contributor

@imbrian imbrian commented Nov 16, 2023

↪️ Pull Request

Removing this check caused a regression when upgrading a project from 2.10.2, where the content key did not yet exist here.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@mischnic
Copy link
Member

Can you figure out what the dependency in question is and why it doesn't have it's parent asset?

For that rewriting to work, both he symbol.set before the if and the if have to execute, otherwise you will get invalid code. So this mostly silences the build failure but probably doesn't actually work at runtime.

@imbrian
Copy link
Contributor Author

imbrian commented Nov 16, 2023

Can you figure out what the dependency in question is and why it doesn't have it's parent asset?

For that rewriting to work, both he symbol.set before the if and the if have to execute, otherwise you will get invalid code. So this mostly silences the build failure but probably doesn't actually work at runtime.

Yeah, I'd like to be able to reproduce this with a test case before considering the PR. Just opening draft, since I spoke to @thebriando about it yesterday.

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.

2 participants