Skip to content

Omit part 'file.g.dart'; When defined in build.yaml #60016

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

Closed
jodinathan opened this issue Jan 30, 2025 · 10 comments
Closed

Omit part 'file.g.dart'; When defined in build.yaml #60016

jodinathan opened this issue Jan 30, 2025 · 10 comments
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-build type-enhancement A request for a change that isn't a bug

Comments

@jodinathan
Copy link

jodinathan commented Jan 30, 2025

Currently, when using code generation in Dart, we need to explicitly include part 'file.g.dart'; in the source file. However, this requirement is redundant when the corresponding .g.dart file is already declared in build.yaml.

I propose an enhancement where the Dart build system automatically recognizes generated files based on build.yaml, eliminating the need for the part directive in the source code.

EDIT:
To make the included part files visible to the end user and prevent unintended part-of declarations, the analyzer could display a link at the top of the original file: "Show part files". Clicking this link would expand a block listing all the automatically added part directives.

@tylandercasper
Copy link

This combined with augments and the "build_runner watch" command would give us 99% of the functionality of macros

@rrousselGit
Copy link

Without macros, augments aren't enough to remove the need for a "part" definition. Although for augments, the syntax is slightly different

So this is useful even with augments

@mateusfccp
Copy link
Contributor

Related: dart-lang/language#1958

@tylandercasper
Copy link

tylandercasper commented Jan 30, 2025

Related: #1958

The main argument against this seems to be the necessity to scan every file to see if it's linked to another one, but if we treat dropping the "part" command as a convenience feature, than we would only need to scan the files that share the same location and root name. IE
main.dart: scans to see if there any main.*.dart files in the same directory, and if they have a "part of main.dart" directive than the "part" is assumed.

EDIT: Since the only side effect of being accidently included is that they have access to the other files private variables, we could also drop the "part of" directive for any of the basename.*.dart files. This would make it easier for users to split up complex files and save us a file scan to look for "part of"

@lrhn lrhn transferred this issue from dart-lang/language Jan 30, 2025
@lrhn lrhn added type-enhancement A request for a change that isn't a bug area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-build labels Jan 30, 2025
@jakemac53
Copy link
Contributor

Explicit part statements are a good thing imo and should stay, for a variety of reasons:

  • You can easily navigate to all the parts of a library from any part of the library.
  • The compilers don't have to do a bunch of work looking around the file system for related files. Today they don't have to do any directory listing in order to discover files.
  • This would significantly introduce the amount of work required by every hot reload (and all incremental rebuilds) - they would have to list the entire file system surrounding all known files and check for any new files on every incremental rebuilds. You can no longer just watch for changes to the known files.
  • build_runner is intentionally separate from the language, we shouldn't be tying anything related to it to the language (such as the build.yaml format).
  • The build.yaml file over declares outputs, unless people are specifically restricting things using generate_for etc. The vast majority of potential outputs are never written.
  • The build.yaml outputs are not necessarily part files at all, they could be new libraries or anything else. You would be baking in a lot of bad assumptions.

Ultimately you would end up paying a significant cost for this on every build which is ultimately much more than the cost of simply writing the part directive. The tradeoff doesn't make sense even if you think it would be a good UX (which I also don't think it would be).

This would also very significantly affect build_runner performance, as in make it orders of magnitude slower. The speed of it currently is already n^2 based on the number of transitive library dependencies, and this would be multiplying the n factor here by some constant for any package that defines these potential outputs - as well as adding a glob node dependency to each of them. It would be absolutely catastrophic for performance.

@jodinathan
Copy link
Author

@jakemac53 In theory, couldn't we have a file (or several) in .dart_tool containing a map of part directives? That way, every time the builders run, it updates this file to specify which files are linked, ensuring that compilers always watch the files they are aware of.

@jodinathan
Copy link
Author

The tradeoff doesn't make sense even if you think it would be a good UX (which I also don't think it would be).

If I had to Cmd+Click the part directive five times to navigate to the generated code, it would be excessive.
On the other hand, I've lost count of how many times I forgot to add the part directive or made a typo.
So, in my opinion, it makes perfect sense not to require the part declaration.

@iapicca
Copy link

iapicca commented Jan 30, 2025

@jakemac53 this was a very fast decision... and not in a good way

@Jure-BB
Copy link

Jure-BB commented Jan 31, 2025

@jakemac53

  • This would significantly introduce the amount of work required by every hot reload (and all incremental rebuilds) - they would have to list the entire file system surrounding all known files and check for any new files on every incremental rebuilds. You can no longer just watch for changes to the known files.

Wouldn't be possible to cache all files in-memory and monitor project directories by listens for filesystem events? That way a new file can be loaded when it's created. No need to list the entire file system surrounding all known files on every incremental rebuild.

  • You can easily navigate to all the parts of a library from any part of the library.

Indexing all files (in-memory) by path+basename should make navigating to all the parts (under same path) straightforward.

Navigating to any part file for partial classes in C# works even though files can be in different directories. IDE lists all files when invoking go to declaration command. Probably by resolving and indexing all namespace+type_names inside each file.

@tylandercasper
Copy link

* build_runner is intentionally separate from the language, we shouldn't be tying anything related to it to the language (such as the build.yaml format).

* The build.yaml file over declares outputs, unless people are specifically restricting things using `generate_for` etc. The vast majority of _potential_ outputs are never written.

* The build.yaml outputs are not necessarily part files at all, they could be new libraries or anything else. You would be baking in a lot of bad assumptions.

These are valid points for not using the build.yaml to derive part files. We don't have to solve the problem in that particular way, but there are other solutions for auto-deriving part file.g.dart especially for super local files.

The tradeoff doesn't make sense even if you think it would be a good UX (which I also don't think it would be).
The product I'm developing is intended for people with little to no programming experience and the "part .g.dart" line is one of the weirdest things to have to explain to them.

  • "Why aren't the generators doing this step themselves?" It's not allowed. For Reasons.
  • "How do I know which extension to use?" You have to look up the documentation for each package to find out.
  • "What happens if there's a typo?" It won't work.
    Dart/Flutter shines in so many ways, but this one aspect is just wierdly messy.

This would also very significantly affect build_runner performance, as in make it orders of magnitude slower. The speed of it currently is already n^2 based on the number of transitive library dependencies, and this would be multiplying the n factor here by some constant for any package that defines these potential outputs - as well as adding a glob node dependency to each of them. It would be absolutely catastrophic for performance.
I don't think I understand the point you're trying to make here. Doesn't build_runner have to look at every file anyway? Is there a way we could implement this that wouldn't be so expensive?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-build type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

8 participants