Skip to content

Fixes for Dart imports #8380

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.

Sign up for GitHub

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

Jump to bottom
Closed
insertjokehere wants to merge 4 commits into from
Closed

Fixes for Dart imports #8380

insertjokehere wants to merge 4 commits into from

Conversation

@insertjokehere
Copy link
Contributor

@insertjokehere insertjokehere commented Aug 20, 2024

Fixes #7609
Fixes #8379

This PR builds on the work from #8259 to:

  • Add a simple test to catch regressions in this area
  • Fix an issue where include chains (a includes b which includes c) are naively represented as a single import in Dart, which doesn't reflect how Dart handles imports. I've adjusted the Dart code generator to include an export statement to match import statements for files in the same namespace to ensure that include chains work in the generated code

@github-actions github-actions bot added c++ codegen Involving generating code from schema dart labels Aug 20, 2024
@fawdlstty
Copy link
Contributor

fawdlstty commented Nov 15, 2024

@CasperN @vaind Sorry to tag you out of the blue. This part of the modification is very important to me. Can you promote the merger of PR?

@vaind
Copy link
Contributor

vaind commented Nov 15, 2024

I'm not a maintainer so I don't have merge permissions

@fawdlstty
Copy link
Contributor

fawdlstty commented Nov 18, 2024

@aardappel Sorry to tag you out of the blue. Can you promote the merger of PR?

aardappel
aardappel previously approved these changes Nov 18, 2024
View reviewed changes
@aardappel
Copy link
Collaborator

aardappel commented Nov 18, 2024

This was merged: #8313

Can you rebase this one, seems there may be some conflicts.

@aardappel
Copy link
Collaborator

aardappel commented Nov 18, 2024

There also seem to be dart codegen changes to the generated files that are not included in this PR

@insertjokehere insertjokehere force-pushed the 8379-dart-recursive-imports branch from 4c76b91 to 115c8e4 Compare November 18, 2024 20:38
@insertjokehere
Copy link
Contributor Author

insertjokehere commented Nov 18, 2024

@aardappel thanks for the review - I've rebased and updated the generated bindings. Happy to squash this down before merge.

@aardappel aardappel enabled auto-merge (squash) November 18, 2024 21:42
@aardappel
Copy link
Collaborator

aardappel commented Nov 18, 2024

There also seem to be dart codegen changes to the generated files that are not included in this PR

This still seems to be an issue

@fawdlstty fawdlstty mentioned this pull request Feb 25, 2025
Closed
@github-actions
Copy link
Contributor

github-actions bot commented May 20, 2025

This pull request is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label May 20, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

This pull request was automatically closed due to no activity for 6 months plus the 14 day notice period.

@github-actions github-actions bot closed this Jun 3, 2025
auto-merge was automatically disabled June 3, 2025 20:36

Pull request was closed

@fawdlstty fawdlstty mentioned this pull request Nov 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aardappel aardappel aardappel approved these changes

Assignees

No one assigned

Labels

c++ codegen Involving generating code from schema dart stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Dart, master] Recursive imports are not respected Dart: missing import in generated file

4 participants

@insertjokehere @fawdlstty @vaind @aardappel