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

V2 App #977

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

V2 App #977

wants to merge 12 commits into from

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Oct 6, 2023

@github-actions github-actions bot added the S-Proposed In the Proposed Stage label Oct 6, 2023
@SergeAstapov
Copy link
Contributor

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

@NullVoxPopuli
Copy link
Contributor Author

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 🤞

@void-mAlex
Copy link

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
Copy link
Contributor Author

I'm a huge fan

@void-mAlex
Copy link

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
Copy link
Contributor

MrChocolatine commented Nov 3, 2023

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
Copy link
Contributor Author

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

I sure hope not.

text/0977-template.md Outdated Show resolved Hide resolved
text/0977-template.md Outdated Show resolved Hide resolved
NullVoxPopuli and others added 2 commits November 4, 2023 11:02
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com>
@void-mAlex
Copy link

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

@mansona mansona marked this pull request as ready for review November 15, 2024 17:35
@ef4
Copy link
Contributor

ef4 commented Nov 15, 2024

Feedback from during RFC meeting:

  • lets add one or more sections to cover package.json (scripts, exports, ember-addon metadata, probably also dependencies)
  • "how we teach this" should explain the general shape of how people are expected to upgrade to this, in terms of "there will be a step-by-step upgrade guide" and/or "there will be a codemod", and does ember-cli-update work.
  • babel config is missing preset env (and how it interacts with targets.js)

@NullVoxPopuli
Copy link
Contributor Author

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
Copy link
Contributor

ef4 commented Nov 15, 2024

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

  • require and requirejs and define and import require, { has } from 'require' are removed.
  • top-level await is added

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
Copy link

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
Copy link
Contributor Author

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

  • new project: ideal, minimal, etc
  • upgrade: we add compat as needed, but try minimal if we can (feature-detection)

@mansona
Copy link
Member

mansona commented Nov 22, 2024

@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
Copy link
Member

mansona commented Nov 22, 2024

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

@evoactivity
Copy link

evoactivity commented Nov 22, 2024

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:
embroider-build/embroider#2183
embroider-build/app-blueprint#115

@NullVoxPopuli
Copy link
Contributor Author

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
Copy link
Member

mansona commented Nov 23, 2024

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
Copy link

evoactivity commented Nov 23, 2024

@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
Copy link
Contributor Author

NullVoxPopuli commented Nov 23, 2024

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
Copy link
Member

mansona commented Nov 24, 2024

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
Copy link
Contributor

ef4 commented Nov 24, 2024

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
Copy link
Contributor

ef4 commented Nov 24, 2024

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
Copy link

evoactivity commented Nov 25, 2024

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
Copy link

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
Copy link
Member

mansona commented Nov 25, 2024

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
Copy link

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
Copy link
Contributor Author

We really need new vs updating detection

@mansona
Copy link
Member

mansona commented Nov 25, 2024

We really need new vs updating detection

this needs to be a separate RFC 👍

@ef4
Copy link
Contributor

ef4 commented Nov 25, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

mansona commented Nov 29, 2024

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
Copy link
Contributor

@mansona I am glad to hear that 👍 .

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Nov 30, 2024

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)

@ef4
Copy link
Contributor

ef4 commented Dec 6, 2024

TODOs for the RFC text:

  • clean up webpack references (the blueprint is opinionated and is about vite, webpack support as an upgrade path is a separate topic)
  • update vite config to reflect newly cleaned up design that is already implemented in embroider now since the discussion here
  • add section about config/target.js compatibility support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Exploring In the Exploring RFC Stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants