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

Get rid of ember install #453

Closed
mehulkar opened this issue Feb 21, 2019 · 26 comments
Closed

Get rid of ember install #453

mehulkar opened this issue Feb 21, 2019 · 26 comments

Comments

@mehulkar
Copy link
Contributor

I’m not entirely clear why we have ember install, but it seems like it just narrows the search for packages by the ember-addon keyword?

Regardless, I think it’s an unnecessary learning hurdle and we should just use npm/yarn to install packages.

(Tangentially related, ember-auto-import has also probably reduced some of the value add of addon packages, meaning that more packages are probably being installed directly now).

@Panman82
Copy link

ember install also runs the default blueprint, which is used quite a bit by addons that wrap other libraries, to install a dependency in the consuming app.

https://ember-cli.com/extending/#default-blueprint

@buschtoens
Copy link
Contributor

...or to create configuration files, etc.

@jrjohnson
Copy link

I agree that ember install is a confusing extra addition to the learning experience, especially since ember-auto-import is going to have more ember devs doing npm install as well.

It also papers over the difference between yarn and npm install. I wonder if we focused on NPM if we could script the blueprint stuff out in a post/preinstall script. I also think that adding dependencies in this way should probably be an anitpattern. If your addon needs the deps then it should install and mange the versions, adding them to my app just gives me one more thing to worry about.

This additional tooling is one of the weirdly heavy parts of adopting ember when you're already an experienced user of other JS frameworks.

@buschtoens
Copy link
Contributor

@jrjohnson

If your addon needs the deps then it should install and mange the versions, adding them to my app just gives me one more thing to worry about.

I am not saying this statement is wrong and I sonewhat agree with it, but sometimes the dependency needs to be defined in the host app, like with peer dependencies.

@mehulkar
Copy link
Contributor Author

Could running the blueprint also happen with npm postinstall?

@rwjblue
Copy link
Member

rwjblue commented Feb 22, 2019

The semantics of postinstall are quite different. postinstall is ran every time you npm i / yarn in the repo, whereas ember install foo runs the blueprint once when you invoke it.

@rwjblue
Copy link
Member

rwjblue commented Feb 22, 2019

Regardless, I think it’s an unnecessary learning hurdle and we should just use npm/yarn to install packages.

How is it a hurdle, please explain? It removes the need for every addon to have examples like:

If using npm run:

npm install --save-dev foo
ember generate foo

If you are using yarn run:

yarn add foo
ember generate foo

it seems like it just narrows the search for packages by the ember-addon keyword?

Ya, I think this is the only thing to change here. Making ember install lodash-es work (and use npm if the project is using npm or yarn if it is using yarn) seems better.

@miguelcobain
Copy link
Contributor

The semantics of postinstall are quite different. postinstall is ran every time you npm i / yarn in the repo, whereas ember install foo runs the blueprint once when you invoke it.

npm docs mention that the postinstall script is ran "after the package is installed": https://docs.npmjs.com/misc/scripts

The way you phrased it makes it sounds like anytime you install something, postinstall script will run. Please correct me if I'm wrong.

@jrjohnson
Copy link

@rwjblue you and @locks convinced me thoroughly that #448 was necessary because NPM was already the community default. In that case I would argue that addons do not need to provide instructions for yarn, anyone choosing to use yarn will be able to do so, and the extra instructions are just noise.

The hurdle is in learning anything here at all. npm install is a known quantity. If I come from anywhere else in JS and node I will probalby be very familiar with this command and its options, if I'm using something like oh-my-zsh there will be a plugin and shorthands for me to use. The new-to-ember devs I've worked with stumble over this. It's not a big stumble, but its a minor annoyance that adds to the opinion that ember is not up to date with the JS world and learning it might not be worth it.

@Panman82
Copy link

@miguelcobain right, but if you delete your node_modules and npm install to re-install everything, then the postInstall hook will trigger again. It may or may not be desirable to have the default blueprint run again in these cases. Since ember install is only needed for the "first" installation, the default blueprint only runs once. If the default blueprint runs upon each npm install, it could potentially wipe out any changes made after the initial install, by re-applying things in the default blueprint.

@mehulkar
Copy link
Contributor Author

mehulkar commented Feb 22, 2019

  1. I wonder how many addons actually define a blueprint that runs on install.
  2. The need to document that users have to run a post installation step is not the end of the world IMO, since it at least brings us back into npm land.

On a higher note, I think it would be really great to align with the larger JS community and treat ember addons more like regular npm packages. The special sauce of an addon is the merge behavior at build time, and we would still have that.

@Panman82
Copy link

Panman82 commented Feb 22, 2019

  1. I wonder how many addons actually define a blueprint that runs on install.

There are 1,077 that have normalizeEntityName(), which is a (unintended) requirement for a default blueprint (I believe anyway).

https://www.emberobserver.com/code-search?codeQuery=normalizeEntityName&sort=score&sortAscending=false

@davewasmer
Copy link
Contributor

  1. I wonder how many addons actually define a blueprint that runs on install.

A significant portion do, and the functionality that the default blueprint provides in these cases is not easily replicated through other means. Just randomly scrolling through the top addons on Ember Observer, at least 3 of the top 25 use a default blueprint.

Personally, I'm quite sympathetic to the idea that dropping things like ember install would lessen the learning curve and unfamiliarity factor of Ember.

However, there are concrete, significant benefits ember install brings to the table as mentioned in this thread. @mehulkar do you have a suggestion of what would fill this gap? Is it just the two step install, npm install and ember g addon-name? If so, I'm not sure what that buys us, and comes at the cost of reduced convenience and consistency.

@Panman82
Copy link

Perhaps there is a way to have a (default) postInstall script run and detect if the default blueprint (if one exists) is applied already, and apply it if not.?.

For example, creating a new addon would result in a pre-defined postInstall script added to the initial package.json file. That script is something that ember-cli provides as part of an addon, and automagically runs the default blueprint if one exists and sets a flag somewhere (not sure where) that it was completed. Upon next run, the script sees that it already had ran the default blueprint and just exists.

@jrjohnson
Copy link

The blueprint process is great for first installs, but breaks on updates so while it is amazing I don't see it as a blocker in it's current state. Maybe moving to a more standard postscript process would open up some options for managing the complication of blueprint changes being applied to the app when updated.

@kellyselden
Copy link
Member

I can agree that the default blueprint is powerful at first, but quickly gets into a weird state.

  • v1 of addon installs setup code
  • v2 of addon rewrites this setup code
  • users updating to v2 never get the new setup code, stuck on old code
  • then you have to know to run ember g my-addon again to get the new stuff.

There's probably a better way to do this, and it might be in-line with the current discussion.

@mehulkar
Copy link
Contributor Author

Not in a position to research this myself until next week, but in the interest of continuing the conversation, how does the “only run generator once” mechanism work? Is there a cache file somewhere or does it detect files in the project? Couldn’t that same mechanism be used by the postinstall script? Couldn’t the ember g be replaced by npx or whatever else is en vogue for scripts bundled with a package?

@kellyselden
Copy link
Member

ember install is more dumb than you're thinking. The time you run ember install my-addon is the only time ember g my-addon is ever run for you. Subsequent yarns or npm is don't touch it. There's no cache involved.

@sheriffderek
Copy link

I like ember install addon-name because it's clear that I'm installing an addon and not just a package. I know nothing of the internals / but I enjoy 'ember' specific abstractions and knowing when I'm in the ecosystem. Just another view. : )

@mehulkar
Copy link
Contributor Author

The time you run ember install my-addon is the only time ember g my-addon is ever run for you.

How is it a hurdle, please explain? It removes the need for every addon to have examples like:

I think it's totally fine and acceptable for Ember users to run npm install foo && ember g foo. If anything, this hidden install step seems undesirable to me. The only way to know that a blueprint was installed is with git right? I am probably fishing for a problem here, but what if the user has a gitignore that manages to exclude some of the blueprints changes?

npm postinstall, albeit doing something different, is at least discoverable by end users since it's a larger JS ecosystem convention.

There are 1,077 that have normalizeEntityName(), which is a (unintended) requirement for a default blueprint (I believe anyway).

Oh nice, good to know! Was not aware of that hook until now.

A significant portion do, and the functionality that the default blueprint provides in these cases is not easily replicated through other means.

Maybe a migration path / community convention could be something like npm run emberinstall?

I get that installing the default blueprint automatically on install is a nice convenience, but I am not sure that copy pasting 2 lines out of a README is that much more of a hassle than copying 1 line. I think it would be one less thing for new teammates to learn and remember.

@runspired
Copy link
Contributor

Fwiw a lot of this discussion has focused on how the postinstall script runs for every fresh install. A non-trivial point lost in that is that every fresh checkout of a project or CI run is going to be an install.

That said it’s probably relatively straightforward for ember-cli to provide a cache file for whether a default blueprint has been run ever before. A default addon postinstall script would ask that cache before running the blueprint. Everything after the initial would need to be a manual “ember g”.

@Panman82
Copy link

I'm wondering if we could take it a step further and provide a public hook for addon authors to decide when the blueprint should run (once, upon every install, after major upgrades, NOT for CI runs, etc.). It should probably default to "once" since that's the current action.

@mikkopaderes
Copy link

mikkopaderes commented May 29, 2019

Just want to note that ember install is usually explained as

  • npm install <addon>
  • ember generate <addon>

But in reality it does this:

  • npm install <addon>
  • npm install (see here)
  • ember generate <addon>

This is why the command may not work in lerna and why I think that the command isn't just a dumb 2-step command as most would imply.

Please correct me if I'm wrong

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

This issue seems to be stagnant now though the underlying issue still stands. I think there's a good case for standardizing the install process and I, personally, don't mind making it two step to make the package manager explicit. However, @mikkopaderes brings up a good concern here. If someone wants to think through this a bit more I don't think it would be hard to write an RFC. Remember, not everything has to be finalized for a draft.

@wagenet wagenet closed this as completed Jul 23, 2022
@NullVoxPopuli
Copy link
Contributor

an RFC was just opened for this today! #831 🥳

@jenweber
Copy link
Contributor

Just a clarification, #831 focuses on only npm start and npm test. It does not include ember install.

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