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

Add generalized rootPackageAssetFor method or equivalent. #78

Closed
jakemac53 opened this issue Mar 7, 2016 · 11 comments
Closed

Add generalized rootPackageAssetFor method or equivalent. #78

jakemac53 opened this issue Mar 7, 2016 · 11 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@jakemac53
Copy link
Contributor

You can't output files in other packages, but it is common to want to output a file which corresponds to a file in another package (maybe its the compiled js file, or a metadata file). Sometimes these files can just be published with that package, but there are a lot of situations where that might not be the ideal case, and it makes sense for the application package to do all the code generation.

My proposed solution is to add the following as methods on BuildStep (which has a package graph representation):

/// Gives a path corresponding to [original] in the local package.
AssetId rootPackageAssetFor(AssetId original);

// Should this return a Uri or a String?
String assetIdToUri(AssetId id);

// Should this take a Uri or a String for the first arg?
// Relative paths will be relative to [from]. Should this be an optional arg?
AssetId uriToAssetId(String uri, AssetId from); 

The rootPackageAssetFor method would return a path like lib/$someSpecialFolder/${original.package}/${original.path} where someSpecialFolder is a special folder name that we decide to reserve for this purpose.

@jakemac53
Copy link
Contributor Author

cc @kegluneq @kevmoo @nex3 @munificent

@jakemac53 jakemac53 changed the title Add generalized localPackagePathFor method or equivalent. Add generalized rootPackageAssetFor method or equivalent. Mar 7, 2016
@kevmoo
Copy link
Member

kevmoo commented Mar 7, 2016

Love the general idea. We should discuss...

@nex3
Copy link
Member

nex3 commented Mar 7, 2016

So rootPackageAssetFor() actually causes a new asset to be copied over?

@jakemac53
Copy link
Contributor Author

It just gives you an AssetId. It should probably be renamed to rootPackageAssetIdFor? Or maybe we can come up with something shorter/better.

@nex3
Copy link
Member

nex3 commented Mar 8, 2016

But if it doesn't copy anything over, doesn't that mean the AssetId refers to a file that isn't there?

@jakemac53
Copy link
Contributor Author

The asset may or may not exist, this method could be used by the Builder which is creating the file to get the AssetId for the asset it is going to create, or by some later stage to read in that asset.

@nex3
Copy link
Member

nex3 commented Mar 8, 2016

It seems confusing to me that the system would provide a function that canonicalizes a particular location for external packages without providing corresponding infrastructure to actual create those assets.

@jakemac53
Copy link
Contributor Author

It does provide the infrastructure, through BuildStep#writeAsString, which takes an Asset.

@jakemac53
Copy link
Contributor Author

Maybe a more full example would help, lets say you want to produce a metadata file for all dart files, but you don't want to publish that metadata with each package. You might have a Builder with a build method that looks something like this:

Future build(BuildStep buildStep) {
  var metaId = rootPackageAssetIdFor(buildStep.input.id).addExtension('.meta');
  var metaContents = getMetaFor(buildStep.input.stringContents);
  var metaAsset = new Asset(metaId, metaContents);
  buildStep.writeAsString(metaAsset);
}

Then a later Builder could read in those meta files in its build method:

Future build(BuildStep buildStep) {
  var metaId = rootPackageAssetIdFor(buildStep.input.id).addExtension('.meta');
  var meta = await buildStep.readAsString(metaId);
  /// etc
}

@nex3
Copy link
Member

nex3 commented Mar 8, 2016

Okay, that makes sense. I think it's fine as long as you're careful to explain that it's just a convention, and provide an example like the one above.

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed enhancement labels Sep 14, 2016
@jakemac53
Copy link
Contributor Author

closing as I don't think we actually need this any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants