pnpm dlx ember-vite-codemod@latest by NullVoxPopuli · Pull Request #100 · btecu/ember-select

pnpm dlx ember-vite-codemod@latest by NullVoxPopuli · Pull Request #100 · btecu/ember-select

Was going to debug this: emberjs/ember.js#20991

but saw that the debugging experience in the test-app was pretty rough.

All this PR does is run the codemod (one-shot), and no further work was done on this branch.

Choose a reason for hiding this comment

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

Are there any changes needed outside of test-app?

How is this making debugging better?

		import config from 'test-app/config/environment';
		import config from "./config/environment";

		import compatModules from "@embroider/virtual/compat-modules";

Choose a reason for hiding this comment

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

Not sure if this is some blueprint or the codemod, but Ember.js convention for a while has been to use single quotes in the JS / TS files

Comment on lines +6 to +13

		const {
		compatBuild
		} = require("@embroider/compat");

		module.exports = async function(defaults) {
		const {
		buildOnce
		} = await import("@embroider/vite");

Choose a reason for hiding this comment

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

Weird that these statements are not on a single line, also ' instead of "

		@@ -1,10 +1,17 @@
		'use strict';

		'use strict';;

Choose a reason for hiding this comment

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

What's with the double ;;?

		"./tests/": "./tests/",
		"./": "./app/"
		}
		} No newline at end of file

Choose a reason for hiding this comment

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

New line

		'--remote-debugging-port=0',
		'--window-size=1440,900',
		].filter(Boolean),
		'use strict';;

Choose a reason for hiding this comment

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

Again, ;;

Choose a reason for hiding this comment

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

you run lint:fix :p

Choose a reason for hiding this comment

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

I tried, but I think the lints are misconfigured a bit, because config/environment.js lints are matching app/config/environment.js which is ESM, unlike config/environment.js

		'--window-size=1440,900',
		].filter(Boolean),
		'use strict';;
		if (typeof module !== "undefined") {

Choose a reason for hiding this comment

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

Does if (module !== undefined) not work?

Choose a reason for hiding this comment

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

idk -- it's what's in the blueprint tho. module doesn't exist in the browser

Are there any changes needed outside of test-app?

nay

How is this making debugging better?

ember-source and library code isn't minified -- sourcemaps work

@NullVoxPopuli CI is failing with the exact some error, hopefully this change helps you in debugging locally.

How do you get it to fail locally?

How do you get it to fail locally?

You have to run one of the versions that fail which right now includes the latest Ember.js version too:
pnpm test:ember-compatibility ember-release

I have not -- sorry I forgot about this 🙈

I have not -- sorry I forgot about this 🙈

Are you planning on investigating the underlying issue?

Found out this doesn't reproduce in firefox!

So,

vite build --mode development
vite preview - visit /tests/
- firefox passes
- chrome has the failure you see in CI

Since discovering this is a chrome-related issue, this also reproduces via just pnpm start, and we don't need to care about the test environment.

This is good because this makes debugging easier.

Interesting finding. So you think the issue might be related to Chromium?

It could be that it's a timing issue difference between the browsers -- but regardless, I think it's silly to try to render while we're destroying, so I think I may have a fix for ember-source 6.8 -- gonna try it out over in that test suite, after fixing this issue via patch locally

🤔 how did floating pass and regular did not

the lockfile still had 6.8.1 🤦

ah emborider optimized and safe aren't needed scenarios anymore

@NullVoxPopuli good news, that fixed the ember-* runs. Bad news, the embroider-* runs are failing.

Bad news, the embroider-* runs are failing.

The scenarios were invalid -- I just removed them.

The test-app is now the strictest possible, and because ember-select has no build behavior, you don't need to test against different environments.

Vite is using embroider, so you don't need separate scenarios for it