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

Pod blueprints for generators #1619

Closed
jakecraige opened this issue Aug 9, 2014 · 13 comments
Closed

Pod blueprints for generators #1619

jakecraige opened this issue Aug 9, 2014 · 13 comments

Comments

@jakecraige
Copy link
Member

just opening this here for some discussion on having generators for the pod structure. Since it's supported we should likely have the option of the default generators supporting this structure. With that said, the question is how?

Proposed solutions:

ember g controller my-controller --type=object --structure=pod
ember g pod-controller my-controller --type=object

it could even be a setting in .ember-cli and then the normal generator commands would work and the blueprint would have to implement a normal structure and a pod one.

ember g controller my-controller --type=object

My thoughts
I'm leaning towards number 2. It's clear in exactly what it's doing and doesn't confuse blueprint options with the style of structure you want. It should be as easy as adding some more blueprints and no unnecessary code debt or complexity would be added.

Thoughts?

@stefanpenner
Copy link
Contributor

--structure pod shorthand -s pod seems nicest.

@peterchoo
Copy link
Contributor

I've just started looking at pods, and wanted a generator for it. The way I thought it could work would be something like ember g pod route-path which would generate the folder structure, and give you the default controller, route and template.

I did ember g blueprint pod, and created the following folder structure in files/

files/
files/app/
files/app/__name__/
files/app/__name__/controller.js
files/app/__name__/route.js
files/app/__name__/template.hbs

This requires the name passed to the generator to include the pods folder prefix, and then the route folder names: ember g pod pods/users - A sub-route would use ember g pod pods/users/index or ember g pod pods/users/user etc.

It would be nice if the generator was aware of the applications podModulePrefix, and would only run if this has been set sensibly, and use the value after the first /

It would be interesting to explore the options you could pass to the pod generator, --no-controller, --no-route, --no-template, maybe. Having some way to include or exclude particular files from the generation.

@jgwhite
Copy link
Contributor

jgwhite commented Aug 21, 2014

I had a great conversation with @trabus on IRC about pod support for generators. I believe he’s made some progress already.

@trabus
Copy link
Contributor

trabus commented Aug 21, 2014

I've just been experimenting with ideas, nothing quite working yet, but I noticed the _installBlueprint method used by the resource blueprint, and the way it makes use of existing blueprints (model and route). The other thing I'd noticed was the mapFile method in the blueprint model, and how it was used for mapping the filenames.

I was thinking that mapFile could be expanded to allow other mappings, and that we could use installBlueprint to handle the pod module generation. This would allow a pod blueprint that simply handles (or receives) the mapping setup (defining path and filenames) and then calls installBlueprint. The mapFile method could take the folder to transform, then an object with all the mappings to apply, so it wouldn't be limited to just __name__.

The blueprints would require some file folder renaming, so the folder/path name and the file name were both tokens, and could be interchangeable for both structures:

files/app/__path__/__filename__.js
files/tests/unit/routes/__testname__-test.js

We'd need to do some sort of checking to make sure files aren't overwritten, but this would allow the core blueprints to be reused for both standard modules and pods. It would also allow pod based nested routes to be generated by defining the path as suggested by @peterchoo. We could also make use of the podModulePrefix in defining the path name, so all you would need is:
ember g route users/user -s pod.

I really like the -s pods idea as well, calling the type of blueprint you want makes more sense to me than calling a pod blueprint. We could just setup the options for generating the pod, then call installBlueprint on the pod blueprint, which would then call installBlueprint on the original blueprint. The other nice thing about this, it allows you to only support pods for the blueprints that make sense in pods. Models for instance, don't really belong in a pod structure IMO.

That's pretty much what I have so far, I was going to start writing some tests to support the idea tonight at the Seattle Ember meetup.

@trabus
Copy link
Contributor

trabus commented Aug 22, 2014

I've got a rough pass at the mapFile rewrite, still working on building up some good tests for it, but it works for the existing tests and the basic ones I wrote for it.

Blueprint.prototype.mapFile = function(file, locals) {
  var pattern, i;
  var fileMap = locals.fileMap || {__name__: locals.dasherizedModuleName};
  file = Blueprint.renamedFiles[file] || file;
  for(i in fileMap){
    pattern = new RegExp(i, 'g');
    file = file.replace(pattern, fileMap[i]);
  }
  return file;
};

This pattern allows you to pass a fileMap object in the options for installBlueprint (which still needs to be extracted into a blueprint method), which would allow more than just the default __name__.

@trabus
Copy link
Contributor

trabus commented Aug 22, 2014

I was thinking about what I wrote earlier, and I'm not sure there even needs to be a separate pods blueprint at all. Also the multiple installBlueprint calls wouldn't be necessary either. I'm pretty sure the fileMap option would suffice for transforming the paths of the original blueprint, without any need for modification to the existing blueprint index.js files.

The pod structure setup could be a simple method of the blueprint model that transforms the paths into pod structure and gets executed somewhere towards the beginning of the install method (if the pod structure is passed). The fileMap would be in place already, so then there's no need for installBlueprint, we'd just continue through the install method. Additional setup could be performed in the beforeInstall hook if necessary.

There would be very little modification needed for existing blueprints, and most of it would consist of adding the necessary tokens into the file folder structure (i.e. __path__, __filename__, __testname__, etc.) and setting up the fileMap defaults using the standard locals. For almost all of the main templates that would use pods, the fileMap would likely use the same pattern.

Anyhow, just some more thoughts I wanted to dump before falling asleep. Would love to hear input on whether this sounds like I'm heading in the right direction.

@trabus
Copy link
Contributor

trabus commented Aug 25, 2014

I'm going to create a method on the Blueprint prototype called generateFileMap, which will generate the { __token__ : value } mappings needed to transform the paths into a pod based structure. This will be called towards the beginning of the default install method. Still working out what parameters would be needed to effectively generate the token map.

I am also going to begin writing some tests that will check several permutations of resolvable paths that are generated by this method.

@trabus
Copy link
Contributor

trabus commented Sep 1, 2014

I made some pretty good progress today, I ended up creating a couple methods (and hooks) to generate the fileMapTokens and the fileMapTokenValues. The generateFileMap method zips the two objects together into a fileMap object to be used for the mapFile method. I also created a method to extract the podModulePrefix from app.js for use in generating the paths.

Regarding tests, I ended up getting stuck trying to write some tests, and decided to go forward with the prototyping before I lost the idea. Now that I have something working fairly well, I'm going to write some tests next before I polish it up for a PR.

The work in its current state is here:
https://github.com/trabus/ember-cli/tree/pod_ideas

@trabus
Copy link
Contributor

trabus commented Sep 2, 2014

I found a better solution for the fileMapTokens. Now it simply pairs a token with a function that returns the value. The generateFileMap method now uses the _fileMapTokens method (default and ones inserted through a hook), and creates the tokens and values for the fileMap used in mapFile.

Default __name__, __path__, and __test__ tokens defined in _fileMapTokens:

    __name__: function(options) {
      if (options.pods) {
        return options.name;
      }
      return options.dasherizedModuleName;
    },
    __path__: function(options) {
      if (options.pods) {
        return options.podPath+options.dasherizedModuleName;
      }
      return options.name+'s'; // TODO: use inflector pluralize(options.name);
    },
    __test__: function(options) {
      return options.dasherizedModuleName;
    }

The standard tokens are __name__, __path__, and __test__ (I found that __test__ is needed because __name__ is not consistent between the default and pod structures). Based on the --structure option, the options.pods boolean is set which determines if the pod value or default is returned. The hook allows lots of flexibility in how the token value is defined, as long as it fits the following pattern:

module.exports = Blueprint.extend({
  fileMapTokens: function() {
    return {
      __token__: function(options) {
          // this could be a series of checks and returns
          // as long as it returns a value
          return 'value';
      }
    };
  },

The fileMapTokens method uses the same pattern as locals which merges hook values with the defaults. This allows blueprint authors to create their own tokens and value functions, as well as override the defaults in the fileMapTokens hook.

Some fileMapOptions are defined in locals and passed to _fileMapTokens:

var fileMapOptions = {
    pods: options.pods,
    podPath: options.podPath,
    name: this.name,
    dasherizedModuleName: stringUtils.dasherize(moduleName)
  };

Now that I have something I'm happy with, I'm going to finally write some tests this week, including a suite of pods- generate and destroy tests.

@peterchoo
Copy link
Contributor

+1

I've been following the updates on here, and I've gotta say thank you for your work on something that's gone way deeper than my prototype blueprint :) can't wait to see this in the CLI!

@jgwhite
Copy link
Contributor

jgwhite commented Sep 2, 2014

Sounds like it’s really coming together. Can’t wait for it to land.

@Genkilabs
Copy link

+1
This sounds good. I was just rolling a new pod and thinking a pod blueprint option would be nice. Thank you for working on this. ^_^

@trabus
Copy link
Contributor

trabus commented Sep 8, 2014

I have been thinking about the flag for pods, and unless another structure other than default and pods is expected to come about, --structure=pod is kind of unnecessary. We could simply pass --pod and make it a boolean, which is essentially what I'm doing with the string value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants