Adopting LibrarySourceSink

[simolus3]

[edit by Morgan: documentation is here]

As a builder author, I'm very excited about replacing source_gen with LibrarySourceSink :) Since I saw the documentation comment reference discussions here, I wanted to get started with a few questions:

• built_value adopted this through a separate builder. Is there a strong reason for this, or could the same builder potentially do both (at the cost of applying source_gen|combining_builder even when there's no reason)? E.g., could I read a BuilderOption key to determine whether to emit a part file or use shared parts, or is there something preventing that?
• Should the _br_/ path configurable? I know this only generates part files that can't be imported, but having internal sources directly under lib/ feels wrong to me. With source_gen, we were able to pick our own output directory, could it make sense to make this directory configurable as well? E.g. for one of my packages I want to contain generated code in lib/src/generated/. Could a build.yaml option for this be considered?

[davidmorgan]

Hi @simolus3 :) thanks for the feedback!

The separate builder is required for built_value because built_value uses build_to: cache plus source_gen's combining builder, while adds_to_library: true must use build_to: source.

However, having both, as in the unreleased built_value right now, is not particularly satisfactory: if the old style builder is still configured it still adds its possible outputs to the build, even if unused, causing unwanted cost. I'm considering a way to allow having both in one builder release in a way that becomes free if the old way is not used. Work in progress :)

The other option is that builders switch to the new way and remove the old way; it's not clear to me if that's a viable migration path for everyone. Feedback would be appreciated :)

Re: _br_, that's a good point about internal sources, thanks.

A few random thoughts on that:

• lib/src would make all the part statements longer than they already are, which is a bit annoying;
• for part files, they're not libraries, so nobody can import them anyway;
• we can add carve-outs to the various tools that know _br_ is special; i.e. we can make it a known location like lib/src; so maybe that's the good compromise. If it's user defined you get more choice but probably not those convention-based carve-outs.

Either way I'll give it some thought :)

Thanks.

[davidmorgan]

@simolus3 some more thought re: migration.

I think the best path might be a minor-version-bump generates-both release, as the built_value_generator now; followed by a major-version-bump release that drops the old way.

build_runner has a new internal feature that can make changes to inputs; it's what's used to write or update the part line for you when you use LibrarySourceSink. An obvious next step is to offer this functionality to builders. Then, the major version bump release can find and fix part statements from the previous version--so it will work out of the box :)

[simolus3]

build_runner has a new internal feature that can make changes to inputs; it's what's used to write or update the part line for you when you use LibrarySourceSink

Cool stuff, I already had a quick look at the implementation out of curiosity. Remembering contents at a specific build phase is neat.

An obvious next step is to offer this functionality to builders. Then, the major version bump release can find and fix part statements from the previous version--so it will work out of the box :)

Altering existing assets that don't come from build_runner sounds dangerous IMO, doesn't this turn build_runner watch into infinite fixed-point iterations if a builder does something silly with its input without converging? We also can't safely remove part statements from a particular builder because multiple source generators might be contributing to it (e.g. for a file that has both drift databases and built_value definitions where the user has an updated version of only one of the builders).

Maybe there could be a way for builders to declare "I used to rely on source_gen but now I use shared parts", and then build_runner could perform the part cleanup automatically once it knows all builders are migrated. But that might be too specific...

[davidmorgan]

The feature would only allow altering primary inputs, for example the step generating foo.g.dart can error with a proposed edit to foo.dart. By default the edit is applied and the build restarts; but loops of edits from the same builder are rejected. With --only-check the edit is not applied.

Yes, badly-behaved builders could cause weird bugs :) but hopefully this is workable.

That's a good point re: auto removing the old part statement being harder if there are multiple builders contributing to parts. I think it's possible to make it work by making the new builders run after source_gen|combining_builder, then they can offer the remove only if the file was not written.