Skip to content

Macros: v2 addon - #2779

Open
NullVoxPopuli wants to merge 1 commit into
mainfrom
nvp/macros-as-v2-addon
Open

Macros: v2 addon#2779
NullVoxPopuli wants to merge 1 commit into
mainfrom
nvp/macros-as-v2-addon

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

For compatless apps, this is needed so that the embroiderResolver can resolve the runtime module in runtime mode correctly

todo:

@NullVoxPopuli NullVoxPopuli added the enhancement New feature or request label Jun 26, 2026
@NullVoxPopuli
NullVoxPopuli marked this pull request as ready for review July 13, 2026 16:24
});

test('pnpm test', async function (assert) {
let result = await app.execute('pnpm test');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great that it's testing the expected behaviour of the isTesting() macro in a test 👍 but we need the other side of the equation. We need something that will run the app (not in the tests) and check that some behaviour (hand wave 👋 ) is different from the test mode.

I recommend that you use the same infra that we have for testing the vite dev mode because it will run the vite dev server and you can visit the app and the tests separately and verify the behviour that way 👍

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed. we had no existing infra that I could find that actually looked at the output in-browser. only the CLI.

@NullVoxPopuli
NullVoxPopuli dismissed mansona’s stale review July 20, 2026 18:38

feedback addressed

@NullVoxPopuli
NullVoxPopuli force-pushed the nvp/macros-as-v2-addon branch from bec2a9f to 8a4f724 Compare July 20, 2026 18:59
@NullVoxPopuli
NullVoxPopuli marked this pull request as draft July 21, 2026 15:05
@NullVoxPopuli

Copy link
Copy Markdown
Collaborator Author

back in draft so I can abstract the vite + testem-proxy way of running qunit via vite dev + /tests for this PR

@NullVoxPopuli
NullVoxPopuli force-pushed the nvp/macros-as-v2-addon branch from 1426cb6 to 00e921c Compare July 24, 2026 14:57
},
"scripts": {
"build": "vite build",
"build:test": "NODE_ENV=development vite build --mode development",

@NullVoxPopuli NullVoxPopuli Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without cross-env (or the .env file), this fails on windows if invoked directly

return fromEnv;
}

let candidates = [

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be default puppeteer behavior, tbh

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(is closer to testem)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this exists because I need to visit the app and ask the DOM questions, which I am uncomfortable with forcing testem in that role

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(because isTesting() is not (always) a compiled thing, I need to assert actual runtime output)

@NullVoxPopuli
NullVoxPopuli marked this pull request as ready for review July 24, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants