emberjs / rfcs

RFCs for changes to Ember
https://rfcs.emberjs.com/
791 stars 406 forks source link

V2 App #977

Open NullVoxPopuli opened 1 year ago

NullVoxPopuli commented 1 year ago

Rendered

SergeAstapov commented 1 year ago

@NullVoxPopuli one important thing this has to mention - Ember Engines. Engines are basically apps and they should follow structure of regular apps.

NullVoxPopuli commented 1 year ago

It's possible "structure" would be a layer on top of this. Like, "structure" is a convention would an overlaying tool could enforce / provide ergonomics on top of. I plan to make a prototype of what I mean next week :crossed_fingers:

void-mAlex commented 1 year ago

I would propose that v2 apps behave like astro islands by default, meaning that you can have multiple apps on a page at once without each of them bringing their own runtime along

this would turbo charge the concept we have today as 'engines' as well as fit well into the desire to have a clear way to boot such an app

both webpack and vite have plugins that support module federation which is one way to achieve this goal if there is appetite for this idea, I am more than happy to help push it forward

NullVoxPopuli commented 1 year ago

I'm a huge fan

void-mAlex commented 1 year ago

also here is a mind blowing fact https://github.com/mitchlloyd/ember-islands existed since Feb 21, 2015 this just proves (yet again) that all software development is just a cycle :)

credit to @kategengler for knowing that addon exists

MrChocolatine commented 1 year ago

this just proves (yet again) that all software development is just a cycle :)

Yeah and in a few years the trend will be to switch back from single-file class/template to separate class and template files.

NullVoxPopuli commented 1 year ago

back from single-file class/template to separate class and template files.

I sure hope not.

void-mAlex commented 1 year ago

we should consider how we have tests running in v2 format currently the /tests path in the browser is restrictive in many ways to code that tries to run with constraints of relative paths to where the app is built it's not feasible to require the app to know the pathname it will be deployed to, at build time (not to be confused with domain name or cdn which is a different consideration)

one problem space I see is with Worker (web workers; service workers etc) that can use importScripts() the path to the scripts can be relative and with /tests path not linking back to the correct place any such work fails it's also not easy to catch these scripts from a build tooling perspective as they can be dynamically build and run at runtime in form of blobs (and importScripts behaves like require in node), which is an increasingly common way to run untrusted code off of the main thread in an isolated way

one consideration would be to have the test runner and app in completely separated server instances and do the work to wire up the tools to get a similar level of productivity you get today from tests - but how that looks exactly I'm not sure

ef4 commented 2 weeks ago

Feedback from during RFC meeting:

NullVoxPopuli commented 2 weeks ago

Is type=module in scope for apps? I think it should be, but wondering what considerations there currently are (or if this is implementation detail territory)

ef4 commented 2 weeks ago

Another thing I think this should cover: what changes about the "authoring format" of app code. Specifically:

I think you actually need almost everything (minus the optional feature flag) that's mentioned in https://github.com/emberjs/rfcs/blob/strict-es-module-support/text/0938-strict-es-module-support.md. Possibly you should just steal the "how to we teach this" from that RFC.

void-mAlex commented 1 week ago

We should make an extra effort to communicate that v2 app is not Polaris edition and that has more pieces in the how we teach this section

NullVoxPopuli commented 1 week ago

for clarification, is v2 app about the blueprint, or just the mechanics?

in part, because I want our upgrade vs new project story to be way different than it is today

mansona commented 1 week ago

@NullVoxPopuli this is specifically about the default blueprint that you get when you run ember new, and just because we have all the compat stuff in there doesn't mean that it's catering for people upgrading. We want people to not have a bad experience when they run ember new and then install a classic addon

Also for the record I added this line in the RFC specifically for this reason:

Any discussion about a minimal β€œcompatibility-free” blueprint should happen in a later RFC

I'm happy to talk about ideas for a minimal blueprint, but that should be a different RFC and should not hold back this one πŸ‘

mansona commented 1 week ago

@ef4 I think I've updated this based on all your feedback πŸ€” could you check if it covers everything you wanted to see?

evoactivity commented 1 week ago

Could the vite package be made a little more ergonomic, so by default the vite config is as simple as possible, but people could still modify it if they needed to.

@embroider/vite could export a function named ember which returns the plugin array. The other named exports would still exist so people could customise if needed.

We could also maintain the extensions list as part of @embroider/vite.

import { defineConfig } from 'vite';
import { ember, optimizeDeps, extensions } from '@embroider/vite';
import { babel } from '@rollup/plugin-babel';

export default defineConfig(({ mode }) => {
  return {
    resolve: {
      extensions,
    },
    plugins: [
      ember(),
      babel({
        babelHelpers: 'runtime',
        extensions,
      }),
    ],
    optimizeDeps: optimizeDeps(),
    server: {
      port: 4200,
    },
    build: {
      outDir: 'dist',
      rollupOptions: {
        input: {
          main: 'index.html',
          ...(shouldBuildTests(mode)
            ? { tests: 'tests/index.html' }
            : undefined),
        },
      },
    },
  };
});

function shouldBuildTests(mode) {
  return mode !== 'production' || process.env.FORCE_BUILD_TESTS;
}

This is similar to how other frameworks tackle this, an example can be seen in svelte where their svelte() function returns an array of their internal plugins. https://github.com/sveltejs/vite-plugin-svelte/blob/main/packages/vite-plugin-svelte/src/index.js


edit:

Looking at other plugins and the vite plugin api I think it might even be possible to get it down to be as simple as

import { defineConfig } from 'vite'
import ember from '@embroider/vite';

export default defineConfig({
  plugins: [ember()],
})

edit2: turns out the above is possible I've just tested this in the latest blueprint

// simplify.mjs
import {
  resolver,
  hbs,
  scripts,
  templateTag,
  compatPrebuild,
  assets,
  contentFor,
  optimizeDeps,
} from '@embroider/vite';

import { babel } from '@rollup/plugin-babel';

const extensions = [
  '.mjs',
  '.gjs',
  '.js',
  '.mts',
  '.gts',
  '.ts',
  '.hbs',
  '.json',
];

export default function ember() {
  return [
    {
      name: 'vite-plugin-ember',
      enforce: 'pre',
      async config(config, configEnv) {
        return {
          resolve: {
            extensions,
          },
          optimizeDeps: optimizeDeps(),
          server: {
            port: 4200,
          },
          build: {
            outDir: 'dist',
            rollupOptions: {
              input: {
                main: 'index.html',
                ...(shouldBuildTests(configEnv.mode)
                  ? { tests: 'tests/index.html' }
                  : undefined),
              },
            },
          },
        };
      },
    },
    hbs(),
    templateTag(),
    scripts(),
    resolver(),
    compatPrebuild(),
    assets(),
    contentFor(),

    babel({
      babelHelpers: 'runtime',
      extensions,
    }),
  ];
}

function shouldBuildTests(mode) {
  return mode !== 'production' || process.env.FORCE_BUILD_TESTS;
}
// vite.config.mjs
import { defineConfig } from 'vite';
import ember from './simplify.mjs';

export default defineConfig({
  plugins: [ember()],
});

Relevant PR's: https://github.com/embroider-build/embroider/pull/2183 https://github.com/embroider-build/app-blueprint/pull/115

NullVoxPopuli commented 1 week ago

I'm a big fan of that. like.. visually that is so much better.

right now our vite config "looks complicate", because there are so many plugins, etc

mansona commented 1 week ago

So... @evoactivity we thought about this near the beginning of the process, it might be nice to simplify things but we went back and forth for 2 main reasons:

  1. Most of the plugins represent legacy build things that it should theoretically be reasonable for an Ember app to have a different set of plugins
  2. we're moving away from the ember-cli way of doing things and we see the fact that surfacing things to the user is a good thing

It's totally possible that we probalby went a bit far on the "show everything to the user" front so there could be a case for a smaller set of plugins or a neater vite config πŸ€” but I have to say that the tiny one-line config that you're demoing makes me feel uneasy πŸ™ˆ

Another thing that came to mind while writing this comment is that as people upgrade using ember-cli-update and the blueprint changes the config I feel like having a diff might be a good thing, what do you think? On the flip side if people aren't using ember-cli-update then they won't get any updates to the config and it's better to bundle things from @embroider/vite because then just updating the package would get the new stuff. I don't know if ther is a certain right way to go here 🫠

I've added this point to the agenda for the next tooling meeting so we can discuss it πŸ‘

evoactivity commented 1 week ago

@mansona I don't think it's just about it being nice. I think it aligns us with the wider ecosystem, aligns us with vite's promise of "Vite is opinionated and comes with sensible defaults out of the box", exposing everything like this basically makes vite feel like webpack, complex and full of stuff I need to understand and configure. Ultimately I think encapsulating that complexity ensures the first impression of ember is that we are at least on par with the other frameworks. Exposing everything upfront I feel makes us feel janky and complicated, especially if I have expectations from using vite with other frameworks and this is what I see when I create a new app.

As as user I don't care about how my app is built, just that it is, and that it is done fast. If I'm one of the small percentage of users who actually needs to customise the build behaviour then I can learn the build tool itself through it's docs and we can document the ember() black box so people can put together a customised pipeline if they need to, but that should be a small percentage of users. This is especially true if we want people to see ember as a viable hobby framework and not just something for big teams with big apps.

I'll address your points individually.

Most of the plugins represent legacy build things that it should theoretically be reasonable for an Ember app to have a different set of plugins

I think there would be better ways to turn off legacy build features as a user than being presented with the full ember vite config from the get go. The RFC also says

this RFC instead is going to propose a new blueprint that has all the compatibility plugins turned on so that it is easiest for most people to upgrade to

So the hows of that can be discussed as we come to it.

we're moving away from the ember-cli way of doing things and we see the fact that surfacing things to the user is a good thing

I don't think this is the ember-cli way of doing things, I think it's the JS and Vite way of doing things. If you look at the wider ecosystem of frameworks on vite this is how they all configure their frameworks. As a user I would expect the same of ember that I have seen in other frameworks, and seeing the current config would make me question why ember is so much more complicated than svelte, vue, react or even angular (when using analog).

Another thing that came to mind while writing this comment is that as people upgrade using ember-cli-update and the blueprint changes the config I feel like having a diff might be a good thing, what do you think?

I think ember-cli-update is an emberism that we should look at as being unnecessary in the future. People use it today because upgrading ember is not always simple, and it's not always simple because we need to make changes to our setups based on blueprints. Personally I don't use it at all. When I am upgrading in other frameworks I just need to update my dependencies and I get the new stuff, I want that same experience in ember.

If we are telling people to use ember-cli-update it's because we don't want them to care about their build system, we tell them, "hey don't worry about it, let the tooling figure out that part and you just concentrate on your app". I believe that in itself highlights why we should hide this complexity from the user. We already try to hide the complexity, but just in a complex way that includes writing a whole tool to do it for them that has a learning cost as a user and maintenance cost as an ember maintainer.


It may not seem like a big deal but I see this new blueprint and polaris future as a big opportunity to tell the world ember is really nice to use, you don't need to learn a million things, we've simplified and we feel we surpass everyone else for the DX of building your app big or small. It's things like this all add up to making us feel clunky and heavy compared to the rest of the ecosystem.

This is how most grug brained devs are going to feel when they take a look under the hood image

In my eyes the future of ember also includes a simplified package.json. Everything a dev would need should exist within a single "ember" package and that's the only ember dependency you need in your package.json to walk the happy path.

import { Component, service, cached } from 'ember';

export default class Thing extends Component {
  @service something;

  @cached
  get simplicity() {
    return 'wins';
  }

  <template>
    <p>But this is another topic altogether, so I won't ramble on</p>
  </template>
}
NullVoxPopuli commented 1 week ago

I'm hugely in agreement with @evoactivity here πŸ’ͺ

Additionally this facade of simplification addresses some of my concerns of "minimal" blueprint vs "being too concerned about upgrade paths via ember-cli-update" -- it allows us to keep ember-cli-update behaviors / expectations while also not caring about "minimal" configs (we can also do light feature-need detection, rather than force config changes in some cases (if needed))

What really nails this approach, is that, if folks want, they can still do things super manually.

mansona commented 1 week ago

Ok so I'm convinced, and for the record I was convinced before I saw the meme in your comment but that just doubly sealed the deal for me πŸ˜‚

I'm still going to leave the agenda item for the Tooling meeting on Tuesday just to make sure that there is full consensus from the team but I will be advocating for this new direction πŸ‘

ef4 commented 1 week ago

I'm all in favor of a smaller config, but we have to do the work to actually make it realistic for the vast majority of apps to stick with it. Probably that means adding some amount of options, and probably it means factoring into more like two or three functions that work together, rather than just one.

For example, a pretty nice outcome would be to have a single function per Ember edition:

export default defineConfig({
  plugins: [
    emberPolaris(),

    // you can delete this once you only use Polaris-era feature:
    emberOctane(),
  ],
});
ef4 commented 1 week ago

Also, some of this could be simpler just by stopping being weird for no reason. For example, nostalgia is not a good reason to make everybody pick a different port than the vite default. πŸ˜†

evoactivity commented 6 days ago

I'm all in favor of a smaller config, but we have to do the work to actually make it realistic for the vast majority of apps to stick with it. Probably that means adding some amount of options, and probably it means factoring into more like two or three functions that work together, rather than just one.

For example, a pretty nice outcome would be to have a single function per Ember edition:

export default defineConfig({
  plugins: [
    emberPolaris(),

    // you can delete this once you only use Polaris-era feature:
    emberOctane(),
  ],
});

This is exactly what I had in mind πŸŽ‰ but rfc said not to talk about turning off compatibility so I didn’t πŸ™ˆ

evoactivity commented 6 days ago

Also, some of this could be simpler just by stopping being weird for no reason. For example, nostalgia is not a good reason to make everybody pick a different port than the vite default. πŸ˜†

I also thought about that but assumed there was a better reason than nostalgia it was setup this way πŸ˜„

mansona commented 6 days ago

nostalgia is not a good reason to make everybody pick a different port than the vite default.

Ok I understand that, but localhost:4200 is baked into Ember developers' brains so people going through an update and then having to change the port that they're working on is not a great experience.

I also don't think it's a big deal to keep 4200 because the terminal is very clear where the app is being hosted

evoactivity commented 6 days ago

I also don't think it's a big deal to keep 4200 because the terminal is very clear where the app is being hosted

I guess that would be the same reason it changing wouldn’t be a problem. Personally I just click the link the terminal spits out.

That said I don’t mind one way or the other if we set the default to 4200.

NullVoxPopuli commented 6 days ago

We really need new vs updating detection

mansona commented 6 days ago

We really need new vs updating detection

this needs to be a separate RFC πŸ‘

ef4 commented 6 days ago

this needs to be a separate RFC πŸ‘

Maybe. But if we're ready to say "yes" to making the config prettier it tells me we're trying to do both jobs in this RFC.

A blueprint with full backward-compatibility for years of Ember patterns is a great thing to have, and it's not a fair fight to side-by-side meme it with blueprints that offer none of that. The vite config is only one small piece of this, it also involves package.json dependencies, the continued existence of config/environment.js, the continued existence of ember-cli-build.js, a lot of stuff in the babel config, etc.

I suspect we need two blueprints, and the easier one to design first is actually the pristine one. Both because it doesn't have to do as much, and because it serves as the gravitational pull for where the backward-compatible blueprint wants to evolve toward.

NullVoxPopuli commented 2 days ago

Does v2 app mean we can remove ember-cli?

Ember-cli brings in noo many dependencies, and makes our install time on slow platforms, such as stackblitz, painful

MrChocolatine commented 2 days ago

Does v2 app mean we can remove ember-cli?

I may be wrong but this sounds like saying goodbye to the traditional "Convention over configuration" deeply embedded within Ember.js.

mansona commented 2 days ago

Does v2 app mean we can remove ember-cli?

The way that we're intending to have the new v2 app blueprint have full ember compatibility we will not be able to drop it for the foreseeable future. We will likely need to keep the rebuild functionality around for a long time πŸ‘

I may be wrong but this sounds like saying goodbye to the traditional "Convention over configuration" deeply embedded within Ember.js.

This is not true in the slightest (so don't worry πŸŽ‰) while it is try that we're planning to rely less and less on ember-cli (the package) we're not planning on removing any of the conventions. A great example is that we are planning to just keep using ember-cli to provide blueprint generation (e.g. ember generate route route-name) but we are also planning to upgrade the very old and hard to work with blueprint system in the near future. We don't need to do that now, we can likely not change any user-facing behaviour, and the new system can be backwards compatible to the old system's blueprints. That sounds like a very Ember way to approach a problem to me πŸ˜‚

MrChocolatine commented 2 days ago

@mansona I am glad to hear that πŸ‘ .

NullVoxPopuli commented 2 days ago

We will likely need to keep the rebuild functionality around for a long time πŸ‘

For the sake of fast installs in webcontainers, could it happen though? Once an app doesn't need compatibility, perhaps for new apps? (Forgoing blueprints, etc)