mansona / ember-get-config

Get `config/environment` from anywhere, even addons!!!
MIT License
65 stars 20 forks source link

Add Embroider support #29

Closed mansona closed 2 years ago

mansona commented 3 years ago

So it looks like the "embroider safe implementation" is actually working, since all the ember-try scenarios are working.

We seem to be hitting a different problem though 🤔 I'm getting an exception The "path" argument must be of type string. Received undefined which seems to be related to calling path.join() with an undefined value.

I've tracked it down to this line https://github.com/embroider-build/embroider/blob/master/packages/compat/src/compat-addons.ts#L195 where dep.name is undefined. From what I can tell this is the "OwnedAddon" (i.e. the ember-get-config addon itself) but I can't quite see why it doesn't have any of the metadata it needs 🤔 and I really don't know what my next steps should be

Here's a screenshot covering what I said about the current state of the app at that error (see the watch values in the top right): Screenshot 2021-10-18 at 10 47 54

NullVoxPopuli commented 2 years ago

@mansona do you have an open issue on the embroider repo? Wondering what needs to be done, todos, etc Trying to get off my custom branch of ember-fontawesome 😅

davideferre commented 2 years ago

@mansona I think you only need to update the @embroider/test-setup version from 0.43.5 to 0.49.0 in your package.json and all the two embroider ember-try scenarios will succeed. As @NullVoxPopuli says this problem is currently blocking the ember-fontawesome release that supports ember 4 😅.

mansona commented 2 years ago

@davideferre thanks that has fixed it! I'd be super curious what the issue was upstream 🙃

mansona commented 2 years ago

Also I'm curious why you said that this was blocking the Ember 4 release? Embroider and Ember 4 are 2 different issues 🤔

ef4 commented 2 years ago

Be advised that embroider 0.48.0 and higher have a working compat adapter that completely overrides ember-get-config's implementation anyway: https://github.com/embroider-build/embroider/blob/f7a4ad0e4d939446ba47c5a0a1adb4a4bb463b9d/packages/compat/src/compat-adapters/ember-get-config.ts

Which is why apps are able to use embroider even when some of their addons have a version of ember-get-config that doesn't work.

We can update the compat adapter so it leaves ember-get-config 1.0 alone. And pretty soon we can port ember-get-config to v2 format, which avoids the whole compat system. I think that is blocked on implementing a part of the v2 addon spec that we haven't had to do yet (the build file support).

davideferre commented 2 years ago

Also I'm curious why you said that this was blocking the Ember 4 release? Embroider and Ember 4 are 2 different issues thinking

Sorry, I don't know why I wrote Ember 4 instead of Embroider, maybe the late hour can explains this error 😅