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

Blueprints Update #477

Merged
merged 12 commits into from Apr 25, 2020
Merged

Blueprints Update #477

merged 12 commits into from Apr 25, 2020

Conversation

@kellyselden
Copy link
Member

@kellyselden kellyselden commented Apr 17, 2019

Rendered

@meirish
Copy link

@meirish meirish commented Apr 17, 2019

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Apr 17, 2019

@meirish Can you try again? I updated the link.

Loading

@kellyselden kellyselden force-pushed the blueprints-update branch from 06ff613 to 29e60fe Apr 17, 2019
@mehulkar
Copy link
Contributor

@mehulkar mehulkar commented Apr 18, 2019

Awesome! I think this would be fantastic. Personally, I am perpetually confused by the various state files in Ember projects... add to that our own build and CI tools, the top level of a project directory quickly becomes intimidating for new users. I would love to see this go in an existing file or even package.json to avoid Yet Another File. It would also be super important for the file to state if it can be changed by end users or not and what effect it would have, so JSON format (sans comments) would be less than ideal, IMO.

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Apr 20, 2019

@mehulkar I've found via further testing that mixing a file that is tracked by the default ember-cli blueprint (.ember-cli, package.json) and modified by ember-cli-update (to update the blueprint metadata), it gets cumbersome. Both processes try to edit the same file. So in theory, ember-cli could alter the .ember-cli file in a way that conflicts with what you have in yours. This would give you a git conflict, then the updater can't read it anymore because it is in a state of invalid JS. I think this further supports a separate file.

Loading

@mehulkar
Copy link
Contributor

@mehulkar mehulkar commented Apr 21, 2019

That makes sense

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Apr 22, 2019

Added sections about complete vs partial blueprints, saving options, and supporting custom blueprint codemods.

Loading

}
```

I vote “ember-cli-update.json”. Since the code will be modified by the updater, it’s easier to reformat JSON. “.ember-cli” I think supports JS, so it would be hard to modify. “package.json” seems like an abuse to put more metadata in there.
Copy link
Member

@rwjblue rwjblue Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, lets drop the other examples and focus specifically on the stand alone ember-cli-update.json.

Loading

Blueprints could be responsible for injecting the state into projects

```js
// my-custom-blueprint/blueprints/my-custom-blueprint/files/.ember-cli
Copy link
Member

@rwjblue rwjblue Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's consolidate all examples in this RFC on the ember-cli-update.json format. I find it pretty confusing to jump amongst 2 or 3 different formats while reviewing.

Loading


**Complete vs partial**

We need a way to denote a project replacement blueprint and a supplemental blueprint. A project replacement removes any sort of default ember-cli blueprint tracking, even if it still looks like a normal Ember project (has ember-cli, ember-addon, etc.). A supplemental blueprint is one that piggy-backs on a complete blueprint and makes certain tweaks to files. Think of ember-cli-mirage's extra project configs, or your company's custom settings on top of a normal Ember app.
Copy link
Member

@rwjblue rwjblue Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to know the difference between complete and partial? I think you are basically saying that we need to know the difference between the original project blueprint (aka the "root" blueprint) and the others so that we know how to build up the comparison diff, is that right?

Loading

Copy link
Member Author

@kellyselden kellyselden Aug 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the time of writing this, I thought I would need to know the difference between the two to do the correct thing, but after implementing most the functionality, I don't think that's true anymore.

Loading

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 26, 2019

A few things here:

  • We should go with a dedicated file for ember-cli-update.json (not merging with .ember-cli file)
  • The file should be in the configured config directory (from package.json's ember-addon.configPath falling back to config/)
  • We should include a version value for the file format itself (a top level key for configVersion or some such), so that we aren't absolutely completely locked down on the final format out of the gates
  • Need to update the RFC to be more explicit about how blueprint options are stored (for example, if I ember new foo --yarn --no-welcome where are those flags stored in this format?)
  • We should workshop the exact format for the blueprints a bit more, I'll try to propose an alternate schema in a separate comment as a way to kick start that convo

Loading

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 26, 2019

RE: format for the blueprints array within config/ember-cli-update.json, I think we should come up with a detailed interface for that format.

Here is what I was thinking (based on reading through this RFC, and looking at some of what was implemented already):

interface BlueprintConfig {
  /**
    Is this the initial project blueprint (e.g. `ember new` or `ember addon`)?
  */
  isRoot: boolean;
  /**
    The name of the blueprint to be ran.
  */
  name: string;

  /**
    The package the blueprint comes from.
  */
  package: string;

  /**
    The version of the package above to use. 
  */
  version: string;

  /**
    The path the blueprint is ran from.
  */
  rootPath: string;

  /**
    The blueprint command line options.
  */
  options?: {
    /**
      The positional arguments to pass to the blueprint.
    */
    positional?: string[];
    /**
      The named arguments to pass into the blueprint.
    */
    named?: {
      [key: string]: string;
    };
  }
}

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Aug 31, 2019

The file should be in the configured config directory (from package.json's ember-addon.configPath falling back to config/)

Good idea. Is there precedence for how a third party could use the same logic to detect this? Filed ember-cli/ember-cli-update#675 and ember-cli/ember-cli-update#680.

We should include a version value for the file format itself (a top level key for configVersion or some such), so that we aren't absolutely completely locked down on the final format out of the gates

Great idea. Filed ember-cli/ember-cli-update#676.

As far as the blueprint schema:

isRoot is probably not necessary.
Are name and package ever going to diverge? Or are you thinking a pretty name to show up in the inquirer prompt instead of package name?
rootPath is for running local file path blueprints right? If so, I'm currently using location, which could be expanded to localFilePath or locationOnDisk.
After experimenting, I don't think options needs to be anything more complicated than an array of strings to be .join(' ')ed, or maybe just a string that is passed as-is.
We need a property that is only used for default ember-cli blueprints, type or defaultType that is either app or addon. It's the only special property needed to tell ember-cli to use ember new or ember addon. Every other blueprint always goes through ember new.

Loading

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Sep 7, 2019

If two addons give you a completely new .eslintrc.js file for example. If you were to do the updates one at a time, you would get an idea what was happening in either case. If we ran them both at once for you, last one wins would be all you would get. I don't think this is something we can solve though.

Ya, agree. Though I think we could solve it by offering a way to run a single blueprint at a time. The default would be the thing we’ve been talking about (run them all in sequence, then diff, then apply), but we would also be able to have a command to run just one.

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Sep 7, 2019

ember new name-here --blueprint addon

woah good to know!

Loading

@runspired
Copy link
Contributor

@runspired runspired commented Sep 16, 2019

What I want is for ember-cli to provide a file that states whether for a specific package+version whether an install command has been run.

It would then be up to addons to use that in a standard npm/yarn post-install hook to decide whether or not to re-run a blueprint/installation/upgrade process. Basically this is "default blueprints" but more universal. We could even provide a little bit of tooling for bootstrapping just such a command into an addon.

I like that this RFC introduces a place in a project under version control for tracking just this!

What I dislike about this RFC is it seems to get us further away from the broader community's tooling expectations. I shouldn't need to know that for project-x I need to install a new dependency with ember install or upgrade it with ember upgrade but for others npm/yarn install or yarn upgrade is correct.

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Nov 13, 2019

I'm struggling to figure out how best to store the options. The goal is to both regenerate CLI commands and match codemod requirements. For example, you want to be able to generate

ember new my-app --no-welcome

but you also want to be able to target a codemod with requirements like

  {
    "versions": {
      "ember-cli": "2.16.0-beta.1"
    },
    "projectOptions": ["app", "addon", "welcome"],
    "nodeVersion": "4.0.0",
    "commands": ["ember-modules-codemod"]
  }

The codemod schema is going to need some work, but the idea is you want to gate the codemod on the presence of options.

If we store CLI arguments verbatim

    {
      "name": "ember-cli",
      "version": "3.2.0-beta.1",
      "options": {
        "positional": [
          "--no-welcome"
        ]
      }
    }

we are not going to match any codemod requirements that don't know about the --no-welcome = --welcome=false rule. This is also a problem for aliases.

We could either mirror ember-cli's argument parsing, which is going to be brittle, or nudge people somehow to store normalised options in the config so that codemods can match them.

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Nov 14, 2019

I'm starting to flesh out more ideas here https://github.com/ember-cli/ember-cli-update/wiki/Config-Schema

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Dec 8, 2019

I added the latest config schema that I've been iterating on here. I think this RFC is now ready to progress. I implemented all the features behind opt-in commands, and I've extensively tested them in the real world. The config schema now feels solid with room to grow.

Loading

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Dec 8, 2019

@kellyselden - Awesome! Would you mind opening an issue to track feedback (and include instructions on how to leverage the new commands and whatnot)? I'd love to kick the tires 😸

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Dec 10, 2019

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Dec 12, 2019

@rwjblue you mentioned storing the config in ember-addon.configPath. Is it expected that an addon would have it's blueprint config stored in tests/dummy/config/ember-cli-update.json? That is not where I would expect because it has nothing to do with the dummy app or testing.

Loading

Copy link
Member

@rwjblue rwjblue commented Dec 16, 2019 — with Octobox

Yes, the top level config is generally for stuff that is used when consumed. tests/dummy/config is where the addon's own configuration should go. I'm totally game to change that around a bit (e.g. add an addon-config/ or something in the top level or remove the top level config/environment.js and saying that config/ is for the addon's own config) but thats a bit of a separate issue.

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Jan 2, 2020

Loading

@NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jan 7, 2020

just saw a demo of this. I'm a fan 👍

Loading

@rajasegar
Copy link

@rajasegar rajasegar commented Jan 22, 2020

@NullVoxPopuli Can you please share the demo link here

Loading

@NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jan 22, 2020

I cannot -- it was an internal / work demo (to/for CrowdStrike)

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Feb 3, 2020

I think it's about time I put this into FCP. I'll ask someone to merge it in a week, or else just close it.

Loading

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Feb 10, 2020

Sorry @kellyselden, I forgot to add the label when you pinged.

FYI - We are moving this into final comment period, and barring new issues being discovered will merge it in about a weeks time.

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Feb 21, 2020

@rwjblue Do you mind merging this?

Loading

@kellyselden
Copy link
Member Author

@kellyselden kellyselden commented Mar 25, 2020

The feature was released, so can someone please merge or close this.

Loading

@locks
Copy link
Contributor

@locks locks commented Apr 25, 2020

Whoops! Merging it :)

Loading

@locks
Copy link
Contributor

@locks locks commented Apr 25, 2020

As always, thanks to everyone that gave us their input to improve the proposal!

Loading

@locks locks merged commit 7deb9a7 into emberjs:master Apr 25, 2020
2 checks passed
Loading
@kellyselden kellyselden deleted the blueprints-update branch Apr 25, 2020
rwjblue added a commit to ember-cli/ember-cli that referenced this issue May 22, 2020
Enables apps and addons to update ember-cli without impacting future
`ember-cli-update` runs.

Without this `ember-cli-update` will use the
current version of `ember-cli` in the `package.json` to determine which
version to attempt to update _from_. Unfortunately, this means that if
you were to update your `ember-cli` version without _also_ running the
full blueprint update, you would have a much harder time of updating
your general project structure in the future.

Support for this format and process was nailed down and merged in
[emberjs/rfcs#477](https://github.com/emberjs/rfcs/blob/master/text/0477-blueprints-update.md).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants