`IsProjected(false)` should not be required in extended types

[glen-84]

Is there an existing issue for this?

• [X] I have searched the existing issues

Describe the bug

When using ExtendObjectType to replace a field, it is necessary to add IsProjected(false) to prevent the replaced field from being projected. According to Michael, this should not be necessary:

You do not need isprojected

Would be a bug Since you have a custom resolver

Steps to reproduce

1. Add an object type like:

   namespace Example
   {
       public partial class User
       {
           public int Id { get; set; }
   
           [InverseProperty("User")]
           public virtual Profile Profile { get; set; } = null!;
       }
   }
   

2. Extend the object type:

   [ExtendObjectType(typeof(User))]
   public class UserExtensions
   {
       public Profile Profile([Parent] User user)
       {
           // ...
       }
   }
   

3. Add to your Query class, with projection:

   public class Query
   {
       [UsePaging]
       [UseProjection]
       [UseFiltering]
       [UseSorting]
       public IQueryable<User> GetUsers(DbContext dbContext)
       {
           return dbContext.Users;
       }
   }
   

4. Note that Profile is still being projected for User.

Relevant log output

No response

Additional Context?

https://hotchocolategraphql.slack.com/archives/CD9TNKT8T/p1652638025694269

Product

Hot Chocolate

Version

12.9.0

[Mephistofeles]

I rely on this behavior to short-circuit dataloaders. For example:

public class City : CodeDefinition
{
    internal int ProvinceCode { get; set; }

    [ExplicitExpansion]
    public Province Province { get; set; } = default!;
}

[ExtendObjectType(typeof(City))]
public class CityExtensions
{
    [BindMember(nameof(City.Province))]
    public async Task<Province> GetProvinceAsync(
        [Parent] City city,
        ProvinceByIdDataLoader dataLoader,
        CancellationToken cancellationToken
    )
    {
        return city.Province ?? await dataLoader.LoadAsync(city.ProvinceCode, cancellationToken);
    }
}

public class Query
{
    [UseProjection]
    [UseFiltering]
    [UseSorting]
    public IQueryable<City> GetCities(
        AppDbContext dbContext,
        IResolverContext context
    )
    {
        return dbContext.CodeDefinition
            .OfType<CityCodeDefinition>()
            .ProjectTo<CityCodeDefinition, City>(context);
    }
}

If loaded by cities query, then Province property is already loaded and dataloader is bypassed. If I load City using another dataloader, then Province dataloader will execute. This way I can have filtering on those properties in some queries and use dataloaders for another queries (I have entities in multiple databases with relationships between them).

[PascalSenn]

@michaelstaib i think this was the reason why we left it this way. The question is a bit what is more common. Should we by default include or by default exclude. At the moment i guess we have @Mephistofeles saying default include and @glen-84 voting for default exclude

This will all be a lot easier with the new projections engine. but this is still a long way to go. Essentially we want to allow to specify the field you inject into the resolver dynamically.

[glen-84]

Just to clarify, I don't yet have a strong opinion either way. I mainly created this issue because Michael considered it a bug, but I haven't really looked into it deeply. I'm still new to HC. 🙂

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.