Add vite support - #434
Conversation
| @@ -0,0 +1,48 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
this file prepares for template coverage, to showcase how it would fit into new architecture to make these changes compatible with #433
| - uses: actions/checkout@v4 | ||
| - name: Install Node | ||
| uses: actions/setup-node@v3 | ||
| uses: actions/setup-node@v4 |
There was a problem hiding this comment.
| uses: actions/setup-node@v4 | |
| uses: actions/setup-node@v6 |
| @@ -20,12 +20,12 @@ jobs: | |||
| steps: | |||
| - uses: actions/checkout@v4 | |||
There was a problem hiding this comment.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v6 |
kategengler
left a comment
There was a problem hiding this comment.
I've started reviewing but will need to continue later.
I'd be okay with dropping support for the old style (telling users in the README to use a particular major).
I also think it would be helpful to explain how the project differs from https://github.com/NullVoxPopuli/testem-code-coverage for vite-based projects.
|
|
||
| ### v2 Embroider addons (Rollup-based) | ||
|
|
||
| For v2 Embroider native addons based on https://github.com/embroider-build/addon-blueprint blueprint: |
There was a problem hiding this comment.
This is not the eventual blueprint. That can be found https://github.com/ember-cli/ember-addon-blueprint
I'd prefer to have compatibility with the official blueprint.
There was a problem hiding this comment.
we do have compatibility with ember-addon-blueprint, it's under Vite support.
this one is for that previous blueprint where we did have support for some time, but readme just didn't explain how to do it.
at minimum we can swap it with Vite docs to make Vite more prominent. not sure it worth to completely remove this section
| [Classic Ember CLI apps](#classic-ember-cli-apps) or | ||
| [Vite-based apps and addons](#vite-based-apps-and-addons) sections. | ||
|
|
||
| ### Vite-based apps and addons |
There was a problem hiding this comment.
We should bump this ahead of the the other instructions since it is the current default for apps and will soon be so for addons
|
|
||
| This matches the [ember-addon-blueprint vite.config.mjs](https://github.com/ember-cli/ember-addon-blueprint/blob/main/files/vite.config.mjs): | ||
|
|
||
| ```js |
There was a problem hiding this comment.
Can we do these examples as diffs so it is easy to see what needs to be added for the addon vs what is already there?
There was a problem hiding this comment.
was thinking the same - will update all examples!
| plugins: [ | ||
| ...(isCompat ? [classicEmberSupport()] : []), | ||
| ember(), | ||
| ...(enableCoverage ? coveragePlugin() : []), |
There was a problem hiding this comment.
Looks like coverageEnvVar is irrelevant for vite projects, then?
| #### Test helper (`tests/test-helper.js` or `tests/test-helper.ts`) | ||
|
|
||
| Instrumentation records hits in `window.__coverage__`, but reports are only written after the browser **POSTs** that payload to `/write-coverage`. | ||
| Wire that up from [`ember-cli-code-coverage/test-support`](https://github.com/ember-cli-code-coverage/ember-cli-code-coverage/tree/master/packages/ember-cli-code-coverage/addon-test-support) in **`tests/test-helper.js`** or **`tests/test-helper.ts`** (same pattern for either extension). |
There was a problem hiding this comment.
This line is confusing -- the demonstration of the use of it below is enough.
| } | ||
| ``` | ||
|
|
||
| If you use **`vite build` + Testem** (output to `dist-tests` or `dist`), the Vite dev server's `configureServer` hook from `coveragePlugin()` only runs under **`vite dev`**, not during a production-style test build. For this case the addon automatically detects `@embroider/vite` in your project's `package.json` and registers `createViteTestemMiddleware()` for you via its `testemMiddleware` hook — no extra `testem.cjs` configuration is required. |
There was a problem hiding this comment.
This needs some clarification
| ## TypeScript integration | ||
| ## TypeScript integration (classic ember-cli / Embroider) | ||
|
|
||
| For Vite-based projects, TypeScript is handled natively by Vite/esbuild — no |
There was a problem hiding this comment.
Lots of implementation details have leaked into these user-facing docs
|
|
||
| * in `ember-cli-build.js | ||
| ``` | ||
| * in `ember-cli-build.js` |
There was a problem hiding this comment.
Is this section still relevant?
| }; | ||
| ``` | ||
|
|
||
| ## Migration from v2 to v3 |
There was a problem hiding this comment.
Not sure this belongs in the README, more like a changelog entry
|
Was wondering if |
Closes #431.
This is pretty large PR because:
The other way I can make this small and potentially easier to review (if anyone wants to do thorough code review):