Add Prototype Vite Packager

[alexlafroscia]

I've been really interested in Vite lately, and wanted to see what it would be like to write a packager for Vite!

I started working on this in a separate repo: https://github.com/alexlafroscia/embroider-vite

Now that it seems like the basics have been worked out here, I decided to open a PR with a cleaned-up version that might be worth landing up-stream. I'll probably keep hacking on it in that repo as well and upstream improvements here as I go!

Packages

@embroider/rollup-plugin-hbs

I borrowed the approach for the Webpack loader to create this. Vite's documentation recommends adopting the rollup-plugin naming if you are not using any Vite-specific functionality in the plugin; Vite plugins are super-sets of Rollup plugins, so we can actually create a Rollup plugin here and it fits perfectly into Vite too! Less to do down the road if someone ever wants a standalone Rollup packager or something.

@embroider/vite

I know that @ef4 mentioned he was working on one a while ago over in #717, but I figured it would still be worth offering up what I had to see if we can land a "real" version of this!

Right now the following is working

• Applying the Babel transforms (by adding @rollup/plugin-babel to Vite)
• Transforming HBS templates (by adding the newly-created @embroider/rollup-plugin-hbs to Vite)
• Handling the commonjs imports that are created by the importSync macro
• Manually moving non-module scripts from the input to the output (which Vite doesn't seem to handle, much like Webpack)
• Copying arbitrary images, etc. from the input to the output (Vite can't see them in the HBS files, so they need to be manually copied)

Currently, the following are not working

• Some require imports are not being transpiled
• Server-side rendering (should be possible, but needs investigation)

And the following need to be implemented in some way

• Testing (probably by using some means of expanding the current tests for the Webpack packager to instead run for each packager)

I think there's a fair amount of overlap in what both Vite and Webpack need Embroider to do for the asset pass-through; I think there's likely some common functionality that could be extracted and shared! Maybe we can talk through what that would look like, as I would love to contribute those changes!

With the current state of this PR, it'll boot an app just fine that uses this as the packager instead of Webpack. It's not nearly as fast as I would like; I need to figure out if/how we can take advantage of running the Vite dev server rather than doing a full "build" on every file change. It would be really interesting if we could come up with a different paradigm for running a packager in "dev" mode, where rather than the packager's build being called for every file change, the dev server would just handle the file changes itself. Maybe something like that would be RFC-worthy?

Relationship to ESBuild

I think between ESBuild, Rollup and Vite, it would make the most sense to create a first-party Vite plugin for Embroider. It's more appropriate for packaging applications, rather than libraries, and itself uses ESBuild or Rollup internally (or at least, supports Rollup plugins). Very little is needed in the @embroider/vite package to connect everything together!

What I'm Looking for Feedback On

• What kind of testing would need to be included to land something like this? I can definitely add test packages that use these packages -- it might be worth thinking through the structure of testing multiple packers in this repo, since everything is Webpack-centric right now.
• Is it worth extracting some shared functionality up front? Or visit that kind of thing later?

[alexlafroscia]

Just for updates: locally, I'm working on a testing strategy. My plan right now is to introduce a PACKAGER environment variable that controls which packager is used in the ember-cli-build.js file. I was hoping to configure this using ember-try but ran into issues there; it seems like ember-try ran into issues with only the env object being configured and no nom packages being swapped in and out.

I'm currently working on this in the test-packages/static-app app, but am running into a transpiration error with Vite not handling something correctly. It's a little hard to tell due to source mapping, but it appears that this file is being handled incorrectly; the bundler appears to be simplifying the file structure and re-arranging all of the sub-modules here into just being part of the index.js file itself, but the way that it's doing that seems incorrect.

View mis-transpiled file

Obviously (void 0) is not itself a function. Chrome does a bad job of tracing the source map to the actual file, but Safari did well enough to point back to a "source" file that at least points in the right direction. All of the JS source maps are a bit messed-up, though... I think this is because the .hbs file transformation lacks source maps, which corrupts the rest of it, but I'm not entirely sure.

Source-mapped file from Safari

I have not come across this with simpler apps that are successfully packaged using Vite.

Maybe it's an issue with that specific module? That test app is using a version of the addon that claims to only support up to Ember 3.8, and the app is running Ember 3.13. It's probably not that, but I'm not sure why this particular module would be having an issue like this. This package also seems to hook up a custom Babel 6 plugin, but ember-cli-babel is providing version 7... maybe that's corrupting the compiled output?

[ef4]

The problem in vertical-collection is probably that it opts out of the standard babel handling and does its own thing instead. This results in embroider seeing AMD instead of ES modules. That actually works under webpack because webpack understands AMD, but it's a bad pattern we'd like people to stop using.

You can workaround by writing a compat adapter for vertical-collection that disables their custom treeForAddon entirely. Here's an example that does a similar thing.

Regarding testing, we have new better test infrastructure implemented within ember-auto-import and @thoov is working on making it available in embroider too. It will make it much easier to write tests that need to run under multiple different packagers. See #756.

[alexlafroscia]

Awesome, thanks for the context @ef4! I'll keep an eye on that PR and see if I can do something to get vertical-collection to work.

That whole package is pretty confusing... it appears that locally, 2.0.0 is installed (based on the package.json having the dependencies of the 2.0.0 version) but the version tag is actually 1.0.0 so... I'm not sure what package is actually being installed here.

[alexlafroscia]

That did the trick for vertical-collection; I'll open a separate PR with that fix

[alexlafroscia]

Current status: I figured out how to get the commonjs plugin de-duplicated by correctly overriding the configuration that Vite provides out-of-the-box. Vite now runs the Babel transformation on the whole module graph. Progress!

Current problems:

1. When ember-paper/services/paper-toaster.js imports ember-get-config (which it itself just re-exporting the default export of static-app/config/environment), the code is ending up in the output bundle as window.require('static-app/config/environment') rather than being re-written to remove the use of commonjs. What's odd to me here is that Rollup is creating the require statement; it's not in the source code, and shouldn't exist in the output either.

2. Some imports are not being re-written by Babel correctly; the original index.js for @ember/test-helpers is being imported rather than the one that Embroider creates and configures Babel to replace the import with.

I have noticed that Babel is opting out of transforming some files produces by the CommonJS plugin; these files have a ?commonjs-proxy query on them, and their contents seem to be ESM formatted. I'm guessing that these files are being created to enable a full ESM graph. It's possible that problem 2 above is caused by this problem; this is my current area of investigation.

Example of what the file path looks like for a ?commonjs-proxy file

The false at the end is part of what I've put in place for debugging, and indicates that that babelFilter will cause this file not to be transpiled. I'm not sure if Rollup has already processed the file to re-write the imports before creating this file, or if we need to somehow run Babel on this file manually, too.

The @rollup/plugin-babel documentation mentions alternate approaches for processing output files rather than input files; I'm not sure what it considers these intermediate files to be

https://github.com/rollup/plugins/tree/master/packages/babel#running-babel-on-the-generated-code

[alexlafroscia]

I pushed up my current state of things, just for reference by anyone following along at home. Spent another hour trying to figure out what is going on with those couple of modules that are not handled correctly... not sure what exactly it is just yet.

One thing I'm wondering, with the ember-get-config thing, is whether Rollup is not handling the re-export from static-app correctly because it's not considered one of that package's dependencies. Maybe it tries to validate that and, seeing as static-app is not a dependency of ember-get-config, re-writes it to the require syntax but does not actually bundle it correctly from there? Just a hunch; needs more exploration. I am thinking about doing something with a compatibility adapter for ember-get-config to test the theory.

[alexlafroscia]

Status update:

• @ef4 gave me some good advice on how to resolve the ember-get-config problem; that problem has been fixed here Add compat adapter for ember-get-config #770
• Some refactoring to the way that we share base packager functionality has been added here, to make review and approval easier through extracting changes unrelated to vite into a different PR Packager Refactoring #765
• I think have some idea why @ember/test-helpers is ending up in the app's JS bundle; it seems like a bunch of modules related to linting tests are being defined in static-app.js which I would assume shouldn't be part of the app's bundle. They depend on a QUnit global being defined. While I haven't found the link to @ember/test-helpers just yet, the fact that test-related packages are being brought into the app's JS graph at all seems fishy to me!

Screenshot of lint test modules in the app JS bundle

[alexlafroscia]

Alright, so it doesn't seem like the issue is the lint package after all.

When handing multiple HTML files off to Vite for building, to seems to create a single vendor.[MODULE_ID].js file that contains some sub-set of the modules in the graph. This file is imported by the "top level" module JS file for both the app JS and the test JS files.

This module includes some JavaScript that is inappropriate for one reason or another. For example:

• The QUnit global is referenced, despite it not being defined for the App JS use-case
• The @ember/test-helpers index.js file for the addon itself ends up in there, which is obviously unwanted

If I tweak the Vite packager so that it only bundles the app's index.html instead, these assets are left out of that vendor JS module and the app builds and boots correctly! So it seems that the issue is that modules are ending up in a shared location that should in fact not be shared.

I'm not sure exactly where the fault here lies. The bundling is performed by Rollup, and I'm surprised that it's creating a set of JS bundles where the app JS includes files that it does not in fact need on it's own. I'm going to dig further into how Rollup is supposed to handle multiple HTML entry points, as I think that should reveal the remaining issue with this packager.

[mattstrayer]

hey @alexlafroscia, thank you so much for your work. This is going to be, imho, a huge win for embroider.

I found this, which may be helpful for the multiple entry point issue you're running into -> https://github.com/rollup/plugins/tree/master/packages/multi-entry

[delaneyj]

What's the status of this PR? I want to try to move an application to vite/embroider and wanted to see if there was a pressing issue stopping this from being merge. Thank you.

[alexlafroscia]

Hey all -- sorry I've been MIA for this PR for a while. I've been busy and once I got my head out of this problem space, it was kind of hard to come back into it again! I'm going to try to get back into this again now that the testing situation seems to have matured.

As for current status, I have the PR rebased and updated so that it should be merge-able once there's a story around testing. Determining how tests will work is what I'm working on now -- I'll be looking at how the Webpack packager works to determine what needs to be done to test the Vite packager as well!

[alexlafroscia]

So I'm seeing a lot of different apps/addon in this directory, all of which are hard-coded against the Webpack packager

https://github.com/embroider-build/embroider/tree/4af550f393b4770cb5f0a78ded29586a8db2d445/tests

It seems like we'll want to expand the testing matrix that scenario-tester is creating to also add the Vite packager to the mix.

I would love some guidance from someone in-the-know (@ef4 @rwjblue @thoov, maybe others?) on how to build the groundwork for different packagers as well.

What I'm imagining is some repo-specific testing utility that detects the installed packager and returns the instance and configuration to use. Something like... this, maybe?

// ember-cli-build.js
'use strict';

const EmberApp = require('ember-cli/lib/broccoli/ember-app');
const { getPackagerFromEnvironment } = require('@embroider/test-support');

module.exports = function (defaults) {
  let app = new EmberApp(defaults, {
    // Add options here
  });

  const { packager, requiredConfig } = getPackagerFromEnvironment();

  return require('@embroider/compat').compatBuild(app, packager, {
    ...requiredConfig,
    skipBabel: [
      {
        package: 'qunit',
      },
    ],
  });
};

Where getPackagerFromEnvironment is able to look at the installed dependencies of the thing (app or addon) under test and determine what packager should be used. This way, scenario-tester can be used to install either @embroider/webpack and webpack or @embroider/vite and vite and depending on which dependencies it finds, return the packager.

My example shows a thought on how configuration to the packager could be provided, too, but I have no idea if that's necessary or a good idea; we just have room in this faux API for that kind of functionality, should it be necessary.

If we like this idea, I could start a separate, prerequisite PR that performs the refactoring required to get the current test suite working this way for the Webpack packager, then build on that to add Vite to this mix for this PR.

I'm very open to other ideas on how we can dynamically determine the packager to build against for each test package in this repo as well! We just need to come up with some way to convert the currently-hard-coded use of the Webpack packager into some kind of dynamic locating, and it seems like we'd want to build on top of the current scenario-tester logic if possible since that's already used to run tests against different combinations of packages.

[ef4]

I think getPackagerFromEnvironment makes sense as you described it.

As far as expanding the test matrix goes, we should do it selectively. A lot of tests are covering things in embroider/compat or embroider/core that don't depend on the packager, so we should avoid duplicating all that coverage. Also, I don't think we want vite vs webpack to multiply against all the different supported ember versions.

[alexlafroscia]

Sounds good @ef4 -- I will try to put something separate together this week that updates some of the tests to load the package from the environment, and then we can work from there on getting the Vite packager tested using that!

[alexlafroscia]

The PR linked above implements the dynamic-packager-loading-for-tests behavior that we discussed here last week. It seems like it works!

Just to keep things moving here, I'll rebase this PR on that branch so we can actually see if the Vite tests would pass under that kind of infrastructure.

[alexlafroscia]

Also, I don't think we want vite vs webpack to multiply against all the different supported ember versions.

I wasn't sure how to pull this off

I tried to use the .skip method in scenario-tester like this

export function supportMatrix(scenarios: Scenarios) {
  return scenarios
    .expand({
      lts_3_16,
      lts_3_20,
      lts_3_24,
      release,
    })
    .expand({
      packager_vite,
      packager_webpack,
    })
    .skip('lts_3_16-packager_vite');
}

but that didn't work; I could skip all of packager_vite but that's not what I wanted. It's not clear how you would skip one of the variants created when using expand multiple times to combine different variants together.

I checked the ember-auto-import test suite and it doesn't seem like .skip is used to skip variants created by multiple uses of .expand. Is this something that could be/should be implemented upstream in Scenario Tester first?

[alexlafroscia]

Whew! Adding Vite to all of the app-template tests takes the number of CI jobs from 89 to 121!

[NullVoxPopuli]

Maybe this is useful? https://github.com/unjs/unplugin

[NullVoxPopuli]

Anything I can do to help get things moving again?

[NullVoxPopuli]

I made this demo here: https://github.com/NullVoxPopuli/vite-demo

everything builds fine, and ember s starts fine (but taking 20s??)
at runtime, I see:

Uncaught Error: Attempted to use test utilities, but `ember-testing` was not included
    e http://localhost:4200/assets/vendor.cc4fd9e8.js:1
    <anonymous> http://localhost:4200/assets/vendor.cc4fd9e8.js:1
vendor.cc4fd9e8.js:1:505581
    e http://localhost:4200/assets/vendor.cc4fd9e8.js:1
    <anonymous> http://localhost:4200/assets/vendor.cc4fd9e8.js:1
    InnerModuleEvaluation self-hosted:2381
    InnerModuleEvaluation self-hosted:2381
    InnerModuleEvaluation self-hosted:2381
    evaluation self-hosted:2332

[mattstrayer]

@alexlafroscia is there anything that the community can help with to get this feature out the door?

[richgt]

Just wanted to throw another comment here - @alexlafroscia - this looks really interesting, and possibly intersects with some work we're doing. Let us know if there's anything we can do to help get this across the finish line.

[x8BitRain]

I asked @alexlafroscia if he could give me any pointers on how to proceed with this but he doesn't know because it's been a while and doesn't have the time to work on it anymore, maybe a new PR is in order??

I got it to build my large test app with embroider 1.8.0 and here are my findings.

templateCompilerPlugin({
  templateCompilerFile: join(this.pathToVanillaApp, meta['template-compiler'].filename),
  variant: this.variant,
}),

meta['template-compiler'] is no longer present on the vite config it seems, it has to link to node_modules/ember-cli-htmlbars/lib/index.js but that dir is sometimes empty during build time, not sure where to get that config from.

It doesn't copy anything from outside the app dir so things required in public will cause errors for not being in the embroider build dir.

Most require imports are not being transpiled (at least on my large test app), as mentioned above, no idea how to fix that one.

[alexlafroscia]

Hey all -- I think I'm going to close this PR and let someone else try to make headway on this. If you can use any of this code to make a Vite packager, please feel free to do so!

As @x8BitRain mentioned, this isn't something I can really move forward with; it's been too long since I wrote the initial work, and I would feel an obligation to maintain this integration going forward that I would not realistically be able to commit to. Sorry to back out of this PR having opened it, but I work on very little OSS these days as it's no longer a part of my job responsibilities.

I hope someone else can make something like this work; Vite is awesome, and I would love to someday see the Ember community in a position to embrace it!

[lifeart]

Linking more radical way to use vite with ember https://github.com/lifeart/demo-ember-vite
To keep vite discusstion thread.

[sandstrom]

@lifeart Interesting!

Do you know if @ef4 has seen this?

As I understood it the general idea with Embroider was to make it easy to use (and outsource) the build logic to popular JS build tools, and avoid building a custom pipeline for Ember (as much as possible). Seems like your demo repo is in the same spirit!

[ef4]

Yes I've seen it. It's the same as several other demos we have shown with ember and vite.

Embroider automatically generates all the messy compatibility code that this demo writes by hand. The demo also defeats code splitting because it registers everything in advance. Recent embroider versions actually skip the registry entirely in the vast majority of cases, because we can use ember's newer template lexical scope.

We expect very soon to be moving people toward inverting the flow of control so that you directly run the tool in question (vite or webpack or rollup) and it has plugins that invoke backward-compatibility behaviors, as opposed to running ember-cli and having the broccoli pipeline invoke the other packager.

[lifeart]

@ef4 code splitting concerns little bit incorrect: lazy routes defined here, but I agree, if component is already registered in global scope, it not gonna be splitted automatically. But, it's possible to have independent bundles of global registered components per route (require manual registry import & initialization on route level)

I would not call part of code as messy compatibility code for this reasons:

• It's clear which parts of addon is actually used in app
• This part of addon initialization could belongs to route, and will be code-splitted
• it registers everything in advance - not true, users manually register only needed parts of addons
• It's easy to refactor, replace any part of addon, without redefining or overriding it in app tree
• It's could be opted out if strict mode is used (no needed to define all in global scope)