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

Code generation creates database table with wrong type #3034

Open
91jaeminjo opened this issue Jun 5, 2024 · 9 comments
Open

Code generation creates database table with wrong type #3034

91jaeminjo opened this issue Jun 5, 2024 · 9 comments
Labels
bug Something isn't working package-drift_dev Affects the drift_dev package

Comments

@91jaeminjo
Copy link

91jaeminjo commented Jun 5, 2024

Describe the bug
I have a class, let's say x that I use in dart. I also have a table in drift that's also named x. in the database.dart file, I use local.x and just x to refer to the database table.

It used to be fine until last December or so, but when I regenerated code this week, the database table code is generated with local.x.

Thank you in advance for your help!

@91jaeminjo 91jaeminjo changed the title Code generation creates database table code with wrong variable type Code generation creates database table with wrong type Jun 5, 2024
@dickermoshe
Copy link
Collaborator

I've had this issue with other code generation tools too.
Could you post the relevant portions of the generated code that is incorrect to help debug this?

@91jaeminjo
Copy link
Author

The db table name is endpoints. The generated code creates the below class Endpoints which should extend Table with Table with TableInfo<Endpoints, Endpoint> but as shown below (as well as all places where Endpoint is referenced), the Endpoint is replaced by location.Endpoint. The location.Endpoint is imported and used in my database dart file.

class Endpoints extends Table with TableInfo<Endpoints, location.Endpoint> {

Please let me know if you need more information.

@dickermoshe
Copy link
Collaborator

@simolus3 It seems that this is not related to the manager API.
I'm surprised this worked in the past.

Could you post the table spec, something reproducible will make it much easier to fix this.

@91jaeminjo
Copy link
Author

Yea I tried disabling generate_manager: false and that didn't fix it.

CREATE TABLE endpoints (
    id INT NOT NULL PRIMARY KEY AUTOINCREMENT,
    name TEXT NOT NULL,
    external_id TEXT,
    uri TEXT NOT NULL,
    description TEXT NOT NULL,
    writable BOOLEAN NOT NULL DEFAULT FALSE,

    UNIQUE (name)
);

This is an example of the db table in the drift file, and I forgot to mention that location.Endpoint is a built value class which also has generated code.

abstract class Endpoint implements Built<Endpoint, EndpointBuilder> {
  int get id;
  Uri get uri;

this class has some checks in the constructor, has a serializer static Serializer<Endpoint> get serializer => _$endpointSerializer;

not sure if these would be relevant..

@dickermoshe
Copy link
Collaborator

Tell me if this is what is happening

endpoints.drift

CREATE TABLE endpoints (
    id INT NOT NULL PRIMARY KEY AUTOINCREMENT,
    name TEXT NOT NULL,
    external_id TEXT,
    uri TEXT NOT NULL,
    description TEXT NOT NULL,
    writable BOOLEAN NOT NULL DEFAULT FALSE,

    UNIQUE (name)
);

endpoints.dart

class Endpoint{
   // ...
}

db.dart

import 'package:drift/drift.dart';
import 'package:app/endpoints.dart' as location;
part 'db.g.dart';

@DriftDatabase(
  include: {'endpoints.drift'},
)
class AppDb extends _$AppDb {
  AppDb() : super(_openConnection());

  @override
  int get schemaVersion => 1;
}

db.g.dart

//... Generated Code 
class Endpoints extends Table with TableInfo<Endpoints, location.Endpoint> {
  // ...
}
//... More Generated Code 

The bug is that we should be using Endpoint that drift created, not location.Endpoint which is your own class.

Is the above correct?

@91jaeminjo
Copy link
Author

91jaeminjo commented Jun 5, 2024

yes. Sorry I could have been more clear. Thanks for your help.

One difference from my actual code is that local.Endpoint is a built value, but not sure if that matters.
I am trying to reproduce this problem in a mini side project right now

@91jaeminjo
Copy link
Author

Ok I get the same problem when I made location.Endpoint just a class not built with built value, and the db.dart doesn't even use the location.Endpoint.

As long as that import '/endpoint.dart' as location is present in the db.dart file, I see this issue.

@dickermoshe
Copy link
Collaborator

@simolus3
I'm not going to have time to fix this for the next 2 weeks (I've got some personal things going on)
I've triaged this bug for now. I just want to make sure that you weren't under the assumption that I've got this covered. I do not.

@simolus3 simolus3 added bug Something isn't working package-drift_dev Affects the drift_dev package labels Jun 6, 2024
@simolus3
Copy link
Owner

simolus3 commented Jun 6, 2024

No worries! This is actually a known problem and a side-effect from trying to be more reliable about finding import aliases. It fixed some cases and made some others worse :(

The problem is that we want drift-generated code to automatically be able to use import aliases (since it inherits those as it is a part file). But for legacy reasons we don't always have the exact definition source file available, only the name (Location in this case). And then we have to guess.

One way to fix this is to use modular code generation. It requires a few changes to your code, but reliably fixes this problem as drift can then generate its own files with imports and we avoid having to guess prefixes.
You can also work around the issue by avoiding the duplicate name (e.g. by defining a typedef EndpointLocatation = Location in another file and then only importing the alias instead of anything directly named Location). But that sucks obviously, and drift should be better here. I'll take a look if we can consider this case when finding prefixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package-drift_dev Affects the drift_dev package
Projects
None yet
Development

No branches or pull requests

3 participants