ember-cli / rfcs

Archive of RFCs for changes to ember-cli (for current RFC repo see https://github.com/emberjs/rfcs)
45 stars 54 forks source link

build target + type #26

Closed stefanpenner closed 8 years ago

stefanpenner commented 9 years ago

some thoughts, leaving them here for future travelers


There is a constant request to expand the number of environment one can use. Although this is perfectly reasonable and internals should (and will support this). I have noticed another similar trend, build targets

Ideally, I would like to not see dev_cordova prod_cordova test_corvoda, dev_electron,prod_electron etc.etc. As this quickly becomes entirely unmanageable and the cost of adding a new target becomes quite large. It also becomes tricky when dealing with addons, that maybe only need to do something in testing of Cordova.

One thought I had, was to split the concept into target and type.

ember build --target electron --type test
ember server --target electron --type test
ember test --target electron
ember test --target electron --server

This could allow us to provide stable public CLI interface, addon hooks, and should help become a good foundation for future innovation.


stuff to explore

cc people who have been doing related work (based on emberaddons.com search): @mike-north @poteto @rwjblue @jakecraige @juanazam @theodorton @brzpegasus @rmachielse

rmachielse commented 9 years ago

Absolutely interesting! I have solved this problem right now by having a separate directory for the app I want to build. Developers can then symlink the contents of the directory with the real app directory, so they can add files or overwrite them.

Doing this with a target sounds more clean. I think type is a very general name, why not just target and environment?

Still, this means that there has to be a way to add or remove files depending on the target. For example by having extra optional directories per target that will be merged into app on build.

stefanpenner commented 9 years ago

Doing this with a target sounds more clean. I think type is a very general name, why not just target and environment?

no reason, i am fine with either actually. My only concern was migration (unless it turns out --target is merely a layer on top, which is likely)

stefanpenner commented 9 years ago

Still, this means that there has to be a way to add or remove files depending on the target. For example by having extra optional directories per target that will be merged into app on build.

ya this seems easy at the addon/broccoli level (assuming we expose the correct info)

juanazam commented 9 years ago

This makes a lot of sense, the current solutions for this issue are quite troublesome. I'm :+1: with using environment instead of type.

brzpegasus commented 9 years ago

@stefanpenner One thought I had, was to split the concept into target and type. This could allow us to provide stable public CLI interface, addon hooks, and should help become a good foundation for future innovation.

That sounds like a good idea. I'd be most interested in what the default Ember CLI commands would do with that information, besides making sure that the app gets built with the right environment configuration (per config/environment.js), and what new hooks this would help expose to addon authors.

If running ember build --environment dev --target foo means the build command could be extended via new hooks to support the specified target without the need for the addon to introduce new commands, that would be pretty awesome. Generally, I find that the core build command or task may work just fine, but I end up creating new commands in order to do something before and/or after the build completes. I'm aware of the preBuild and postBuild hooks, but those are not aware of which command is currently being run (in order to either proceed with custom build steps or skip). We'd need something like preBuildFor(target), postBuildFor(target), or something along those lines.

For example, in ember-cli-nwjs, the ember nw:package command consists of simply running the default build task, then package. Similarly, the ember nw command is the equivalent of doing ember build --watch and running the nw process, but it's a bit more tricky because it needs to spawn nw after the default build task is done, but before invoking the watch promise that never resolves.

I've only just poked around, but I see a similar pattern in ember-cli-cordova's cdv:build command.

Are those the kinds of things you are hoping to provide better support for with the new target and type/environment options?

stefanpenner commented 9 years ago

Are those the kinds of things you are hoping to provide better support for with the new target and type/environment options?

Yes exactly. If you can help assemble detailed scenarios we can likely figure out how to address them with these new hooks.

stefanpenner commented 9 years ago

It seems also useful to make the boot process a public API. The goal would be to parameterize the boot process, allowing different targets to specific everything from attributes, to rootElement to various modes etc.

cc @wycats

stefanpenner commented 8 years ago

this should be ported to a fleshed out RFC

mixonic commented 8 years ago

We discussed this at F2F and I noted the term "target" was similar to "deployTarget" in Ember-CLI Deploy. http://ember-cli.com/ember-cli-deploy/docs/v0.5.x/usage-overview/ Maybe it is a good thing to coordinate on as we make this an RFC @stefanpenner @lukemelia @achambers

achambers commented 8 years ago

We moved from environment to deployTarget as we wanted to keep a clear separation between what was an ember-cli build environment and what was a deployTarget. Would be great to keep this separation clear. :+1:

runspired commented 8 years ago

Only just discovered this RFC thanks to @brzpegasus

I've been formulating ember-cli-platforms as a concept for a long time, and began final sketches heading into implementation this week for work at IsleofCode. We're intending to move fairly fast.

IsleofCode has picked up ember-cli-cordova, has internal stuff used for chrome/firefox extensions, and I've talked with @felixrieseberg @achambers and @brzpegasus about the implementation as it concerns sharing / working with ember-cli-deploy, ember-electron, and ember-cli-nwjs.

I would love additional feedback on the sketch (it's just a readme) from @lukemelia @stefanpenner @mixonic and whomever else is currently pondering this over.

Proposal

stefanpenner commented 8 years ago

--target could easily become --platform, that would also help with the current ambiguity between --target here and --target in ember-cli-deploy

stefanpenner commented 8 years ago

@runspired my feedback is basically, lets not add another top-level command that then implements all other commands. Rather this feels like a mode of the existing commands, triggered by --platform flag.

runspired commented 8 years ago

@stefanpenner I originally envisioned it that way, and prefer it, but unless this functionality goes into ember-cli proper that would seem to involve overwriting many existing commands, which I felt uncomfortable doing.

stefanpenner commented 8 years ago

the idea if this issue, is to explore adding it to ember-cli.

felixrieseberg commented 8 years ago

I did a horrible job of explaining my thoughts to @runspired in a call, so I'll give this another try: At least from a NW.js/Electron perspective, it'll be hard to develop a "good" app without your app being in a platform-relevant state.

In other words, if your app runs fine in a browser, you should probably not use Electron or NW.js. Security implications are huge and potentially super dangerous. Remember XSS? With Electron and NW.js, XSS is suddenly a lot more powerful and can read/write your disk, deploy malware, etc.

There's still very good reason for Ember to have a --deployTarget flag, but we should somehow be aware that we might need to teach Broccoli to think like a compiler (use module foo for platform bar). @runspired had some good ideas around that.

runspired commented 8 years ago

@stefanpenner I would love to add this to ember-cli, if I were to change the commands to use a flag instead, could this proposal become an RFC, and if so, what would the timeline be for moving that RFC forward? I ask because we would like to move forward on this quickly, but obviously quickly is not always in the best interest nor the best approach for tech going into ember-cli itself.

brzpegasus commented 8 years ago

@felixrieseberg

In other words, if your app runs fine in a browser, you should probably not use Electron or NW.js. Security implications are huge and potentially super dangerous. Remember XSS? With Electron and NW.js, XSS is suddenly a lot more powerful and can read/write your disk, deploy malware, etc.

Slack is available both as a desktop app (using Electron), and on the web, with some variations of course. But I can see an app wanting to support both.

felixrieseberg commented 8 years ago

On OS X, Slack is using MacGap, not Electron - pretty much for precisely that reason ;-)

Edit: I obviously get your point though. I'd just be really happy about a platform "state" of sorts - I don't have any opinions about the implementation, but I believe that other cross-platform dev tools have done a fairly good job (thinking about Cordova's platform folders or Visual Studio's deployment target switches).

stefanpenner commented 8 years ago

@runspired I'm able to help you iterate on the RFC to get it to a good place. I don't really have much time for implementation, but maybe small bursts (could use a champion).

runspired commented 8 years ago

@stefanpenner I began converting the README to an RFC here: https://github.com/ember-cli/rfcs/pull/35

runspired commented 8 years ago

If you've got time, I'd love to schedule a google hangout over the weekend or on Monday to do a 30min review and discussion.

brzpegasus commented 8 years ago

@felixrieseberg

On OS X, Slack is using MacGap, not Electron

Interesting. Well, it is using Electron for Windows :stuck_out_tongue: Not sure why it's different.

runspired commented 8 years ago

@felixrieseberg @brzpegasus possibly for the same reasons that I've used nwjs for both platforms before: MacGap does not support windows, and cef can be a huge pain.

stefanpenner commented 8 years ago

@runspired I would love to setup a call, ping me in slack and we can coordinate.

stefanpenner commented 8 years ago

@felixrieseberg brings up great points, let me make a list of them (let me know if i missed something)

each platform:

Given these, building a good and safe experience can be harder then expected.

That being said, I do believe an application could be designed to progressively (and safely) be deployable to multiple environments. The question is, can this be done with the default experience also being a recipe for success. (Which I believe is the primary concern @felixrieseberg raises)

I would love to explore this in more detail, as I suspect we want to mitigate :footprints: :guns: to the best of our ability.

Given that, I still believe (someone correct me if I am wrong) but adding --platform to the CLI doesn't change much on this front, other the unifying some tooling fragmentation we see today.

I do feel we should message these risks, and maybe the unified experience gives us a centralized medium for that.

felixrieseberg commented 8 years ago

+1 on all points. This problem is certainly solvable with good documentation and education.

stefanpenner commented 8 years ago

We certainly want to avoid encouraging issue such as https://code.google.com/p/google-security-research/issues/detail?id=693 (enjoy the fun read)

lukemelia commented 8 years ago

My experience has been that the proposed approach is good for apps with mostly identical behavior across platforms and little to no native code. However, I don't think it scales up very well as complexity increases. Take our iOS app, for example, which uses an Ember app in a webview but also has it's own native UI, data access layer, and the like. It is in a Github repo centered around it's XCode project, which pulls in our ember app via npm. Similarly, our android app is Android Studio-oriented and has a dependency on the same ember app. This approach has allowed for no-compromise native development -- we don't need to deal with ember-cli when working in an Objective-C/XCode-centric ecosystem. It also allows for independently releasing platforms with clean version dependencies. e.g. at a given point in time, yapp-android depends on yapp-ember-mobile v2.3.5 while yapp-ios depends on yapp-ember-mobile v2.4.1.

I'm not opposed to having the proposed functionality in Ember CLI or as an addon, but I would not anticipate choosing to use it for apps beyond side-project status.

runspired commented 8 years ago

Talked with @lukemelia on slack, I think we can reconcile these dev flows, will ping back with formulated thoughts once I've mulled them over.

stefanpenner commented 8 years ago

Hey Luke, I would love to set up a hangout to talk specifically about yapp mobile and this. I would love for it to provide value in that scenario as well. I suspect I can be

lukemelia commented 8 years ago

@stefanpenner Happy to discuss. Maybe sometime on Monday and @raycohen and join too? DM me with good times for you.

runspired commented 8 years ago

@lukemelia @stefanpenner I can do Monday or Tuesday easily, Stef already mentioned to me that Tuesday would be better.

stefanpenner commented 8 years ago

Ya tuesday I believe should work (monday i'll be traveling)

stefanpenner commented 8 years ago

an actual RFC: https://github.com/ember-cli/rfcs/pull/35/files