Skip to content
This repository has been archived by the owner. It is now read-only.

ember-cli platform flag and platform support #35

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@runspired
Copy link

commented Feb 12, 2016

Intended to supplant #26
rendered

This was extracted from an addon proposal, and there are areas (testing, how to handle security concerns of some platforms, and how to expose the pipeline to plugins) that would evolve overtime in an addon and are less fleshed out from the perspective of being an addition to ember-cli than they ought to be.

Addon where this is likely to be built initially

**Create a platform.**
```cli
ember platform <type> [-t "<name>"]
```

This comment has been minimized.

Copy link
@Turbo87

Turbo87 Feb 25, 2016

Member

I'm not a big fan of ember platform as a "generate something for me" command, but then again we also already have ember addon which is sort of similar. An "addon" however is something that actually gets generated while it's not exactly a "platform" that gets generated, but a platform specialization and configuration. I'm not a native speaker but maybe we can find a better name for the command?

Also wouldn't it be better to use ember generate platform <name> here since the command description actually already starts with "Generates"?

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

A good point. A key distinction is that a platform does far more than generate test and app files, you also get a platform addon similar to an in-repo-addon and a platform directory (e.g. platforms/cordova-ios.

Another potential distinction is that it may be beneficial to have platform have install behavior as well. If a given platform addon is not already present (say ember-cli-platform-cordova), then ember platform would fetch and install it. Whether this is valuable, I'm less certain, and I'd be comfortable falling back to generate over increasing surface area.

/<type>[-<name>]
```

These "addons" function differently from normal addons, and would be prefixed with `-platform-`.

This comment has been minimized.

Copy link
@Turbo87

Turbo87 Feb 25, 2016

Member

why do we need the prefix? if platforms must have a corresponding config file we could just list the platform config files to determine which ones are "special".

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

They don't necessarily have to have a config file, I think relying on a potentially volatile (and easily deletable) configuration file is going to be trickier than distinguishing between in-repo addons and in-repo platforms in this way.

You can import the corresponding `app` version of a module into the platform addon.

```js
import Foo from '<project-name>/routes/foo';

This comment has been minimized.

Copy link
@Turbo87

Turbo87 Feb 25, 2016

Member

somehow I have the feeling that the implementation of this will be quite complicated. also what if you want to share code between e.g. the two cordova platforms but want a different impl on electron?

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

somehow I have the feeling that the implementation of this will be quite complicated.

This is fairly simple to pull off with a few broccoli funnels, similar to how a Dummy app works with addon and app directories in an addon.

also what if you want to share code between e.g. the two cordova platforms but want a different impl on electron?

Sharing in this way would be neat, but trickier to work out. Potentially we should enable a cordova platform that both cordova-ios and cordova-android would inherit from. E.g., one for platform type, and an additional specialization for name.

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

This is fairly simple to pull off with a few broccoli funnels

Actually I retract that. It's not fairly simple, but nor is it overly complex.

I imagine it would work sort of like this.

When merging the platform with the app, we rename the platform modules to <app-name>, and when a conflict is encountered, rename the app version to app/ from <app-name>/. And the import of that from the platform would be:

import OriginalModule from `app/path/to/original`;

Initially, this results in there always being extra code (e.g. both original and platform version) shipped, but once we have tree shaking (I'd imagine not very far off from the point this would land) that wouldn't matter.

"ember-cli-platform",
// if present, indicates this addon implements deployment hooks
"ember-cli-platform-deploy"

This comment has been minimized.

Copy link
@Turbo87

Turbo87 Feb 25, 2016

Member

why use three different keywords here and not just ember-platform? we could probably just check if the hooks are implemented by the addon or not, right?

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

We can get by with one.

Ostensibly, this is for permissions elevation. In the original conception, ember-cli-platform meant an addon worked with the ember platform command, ember-cli-platform-plugin meant it would work with build, serve and test commands, and ember-cli-platform-deploy meant it would work with the deploy command.

Keeping this distinction probably would let us be a bit faster at booting the commands, but it's definitely not essential.




### Creating a Deployment Plugin

This comment has been minimized.

Copy link
@Turbo87

Turbo87 Feb 25, 2016

Member

Should we possibly limit the scope of the proposal for now and skip the deployment process for now? Do we actually need to adjust ember-cli for that in any way or is this only relevant for ember-cli-deploy?

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

We should limit the scope and leave this to an addon for the moment.

While this is mostly relevant for ember-cli-deploy, it's important to note a lot of base code is going to get repurposed for this project from ember-cli-deploy.


Then an `npm` command script will be generated and installed that proxies use of
`cordova-ios` into the cordova project directory ios, allowing you to easily use
the cordova-cli.

This comment has been minimized.

Copy link
@Turbo87

Turbo87 Feb 25, 2016

Member

I'm not sure I understand what should be generated here...

This comment has been minimized.

Copy link
@runspired

runspired Feb 25, 2016

Author

Projects such as cordova come with their own cli and own commands to run, ember-cli-cordova attempted to proxy these commands through an ember command, but that results in a very large overhead to setting up and running the command.

This would be a simple script which allows you to run that platform's commands from your ember project directory instead of needing to change directories. It's a convenience factor which probably should be taken out of scope and left to an addon.

@kellyselden

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

I didn't see anything about supported platforms. Would ember-cli come built in with any platforms, or just the base platform command with no implementations. Would the current ember-cli system be refactored into a default (implicit) platform of "web-app"?

@Turbo87

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

Would ember-cli come built in with any platforms, or just the base platform command with no implementations.

If I understood correctly ember generate platform cordova --name=ios would try to install ember-platform-cordova automatically and then use it to generate the necessary files

Would the current ember-cli system be refactored into a default (implicit) platform of "web-app"?

Since the usual "web-app" has no other special tooling needed it should probably stay the default. I think it's also still the 90% use case so that should be as simple as possible, which also means not bothering dev with the "platforms" concept as long as they don't need it.

@runspired

This comment has been minimized.

Copy link
Author

commented Feb 25, 2016

@Turbo87 @kellyselden no platforms come built in, the idea is to expose the right hooks and flags so that a platform plugin can be individually developed for each platform that can rely on a standardized setup and process instead of needing a unique DSL and much "guts" code repetition for these things.

Would the current ember-cli system be refactored into a default (implicit) platform of "web-app"?

Yes. End user's (addon app creators) wont see any change in code organization, but running any command without the platform flag would be the equivalent of running it with the "web" flag.

If I understood correctly ember generate platform cordova --name=ios would try to install ember-platform-cordova automatically and then use it to generate the necessary files

This is up for debate. I think it should be the case, but that might be over greedy and lead to an undesirable level of complexity.

@Turbo87

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

This is up for debate. I think it should be the case, but that might be over greedy and lead to an undesirable level of complexity.

I think I would prefer something like this:

ember install ember-platform-cordova
ember generate cordova-thingy android

It's not much more difficult than the proposed command but leverages a lot of the existing code and concepts.

@runspired

This comment has been minimized.

Copy link
Author

commented Feb 25, 2016

I'm going to leave this as a comment as I haven't had time to update the RFC.

For this RFC, we will need to make sure to:

  • permeate the platform through the system

We likely should enable:

  • multiple platforms to be served at once

After conversations with @brzpegasus @stefanpenner @lukemelia and @felixrieseberg, it looks like this RFC has surfaced a number of common addon needs that are larger than the scope here, and potentially could become 3 RFCs.

"middleware hooks"
-> addon hooks for testem options
-> custom testem launchers
-> customize build, and watch including with a custom index.html for the tests

The short and dirty solution for this for the near term is to allow platform plugins to extend or replace tasks more easily.

"install patches"
-> ability to "patch" modify

  • README.md
  • package.json
  • testem.json
  • .travis.yml / .circle.yml
  • environment.js
@runspired

This comment has been minimized.

Copy link
Author

commented Feb 25, 2016

@Turbo87 We would still be able to leverage the existing code, but I do agree with the sentiments of not expanding the surface area. Possibly another way of achieving this elegantly would be.

ember install ember-cli-platform-cordova --name="ios"

Where that flag is passed into the default blueprint, and (I'm pretty sure) that capability already exists.

@Turbo87

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

"install patches"

I don't think there's anything stopping you from doing this right now, there's just no API available for e.g. reading/writing markdown. or are you talking about applying actual patch/diff files?

@Turbo87

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

ember install ember-cli-platform-cordova --name="ios"

but then you need a different command if you want to add a second cordova target... I'd try to keep it as simple as possible. install for installing the addon, generate for generating a target/platform/whatever it's called.

@runspired

This comment has been minimized.

Copy link
Author

commented Feb 25, 2016

@Turbo87 since install runs the default blueprint, we'd be assuming that

ember g platform cordova --name="android"

would actually be the same blueprint.

@runspired

This comment has been minimized.

Copy link
Author

commented Feb 25, 2016

@Turbo87

there's just no API available

Correct. @stefanpenner and I both feel there's value in having an official API available. I've done this in the ember-cli-toolbelts experiment, and would love to expand on that work and give it a nice API.

@Turbo87

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

@runspired I know that it could technically work. I'm just saying that as an end user of such an ember-platform-cordova addon it would be easier to grasp if the commands were separate 😉

@Turbo87

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

there's value in having an official API available.

I don't see how that is the responsibility of ember-cli. IMHO it should give blueprints and/or addons a hook after the files are written so that they can be processed if needed, but writing markdown or something like that seems out of scope for me. there are probably libs on NPM that could do that but I don't think they should be part of every ember-cli install for the few addons that actually want to modify a default readme file.

@kellyselden

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

Since the usual "web-app" has no other special tooling needed it should probably stay the default. I think it's also still the 90% use case so that should be as simple as possible, which also means not bothering dev with the "platforms" concept as long as they don't need it.

@Turbo87 I wasn't suggesting people would have to run extra commands, just that ember-cli internals use the platform system to generate the "web" platform implicitly, similar to @runspired 's comment:

Yes. End user's (addon app creators) wont see any change in code organization, but running any command without the platform flag would be the equivalent of running it with the "web" flag.

@nathanhammond

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2016

IMHO it should give blueprints and/or addons a hook after the files are written so that they can be processed if needed, but writing markdown or something like that seems out of scope for me.

@Turbo87 @runspired See: #36

We're basically discovering that, if we want to keep code out of ember-cli core, we need to carefully design extensibility patterns for each major ember-cli construct. That discovery was echoed again in today's meeting.

@nathanhammond

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2016

At the conclusion of today's team meeting we were unanimous in the opinion that this is the wrong level at which to attack this problem. The underlying problem here isn't that we need ember-cli to support a new type of behavior, but instead that modifications to ember-cli are required to support this type of behavior. It's our goal that something of this nature should be possible to accomplish inside of an addon without requiring the present amounts of copypasta, clever imports, and monkeypatches.

At this point we know very well that it is impossible for us to do much more than make educated guesses as to how people wish to use ember-cli, and who knows what our Angular friends will discover in their world. With that context, our work on ember-cli needs to be responsible for a few things:

  • Making sure that the happy path for users remains consistent, stable, and easy.
  • Maintain focus on the core pieces of ember-cli as opposed to trying to consume responsibility for the entire ecosystem of ember-cli addons.
  • Make it possible for users to rapidly get exciting new features.

In order to achieve the last one we discussed identifying/designing what the core extensibility primitive is inside of ember-cli for each of the main components. This is similar in what was discovered with Broccoli where everything along the way simply receives a tree. It's unlikely that we do as well in creating such a beautifully simple primitive to accomplish the entire functionality, but that should be the measuring stick by which we evaluate our solution for elegance.

With that, @runspired, I motion to close this in favor of a different RFC (or multiple RFCs), yet to be written. If you agree we'll close this PR. 😄

@rwjblue

This comment has been minimized.

Copy link
Contributor

commented Jan 19, 2019

We are working on closing the ember-cli/rfcs repo in favor of using a single central RFC's repo for everything. This was laid out in https://emberjs.github.io/rfcs/0300-rfc-process-update.html.

Sorry for the troubles, but would you mind reviewing to see if this is still something we need, and if so migrating this over to emberjs/rfcs?

Thank you!

@rwjblue rwjblue closed this Jan 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.