Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service injection can't be found in Embroider Optimized #1192

Closed
angelayanpan opened this issue Apr 18, 2022 · 13 comments
Closed

service injection can't be found in Embroider Optimized #1192

angelayanpan opened this issue Apr 18, 2022 · 13 comments

Comments

@angelayanpan
Copy link
Collaborator

angelayanpan commented Apr 18, 2022

getting an error: "Attempting to inject an unknown injection" when running tests with embroider-optimized. The classic ember build and embroider-safe are both testing sucessfully.

repo to reproduce: https://github.com/angelayanpan/test-hftnb-embroider-2

@service('test-hftnb-embroider-2@my-service') myService;

run: yarn test:ember

not ok 3 Chrome 100.0 - [43 ms] - Integration | Component | components/demo-component: it tests
    ---
        stack: >
            Error: Assertion Failed: Attempting to inject an unknown injection: 'service:test-hftnb-embroider-2@my-service'
                at assert (http://localhost:7357/assets/vendor.js:42976:15)
                at Registry.proto.validateInjections (http://localhost:7357/assets/vendor.js:8941:62)
                at FactoryManager.create (http://localhost:7357/assets/vendor.js:8381:35)
                at Proxy.create (http://localhost:7357/assets/vendor.js:8106:20)
                at CurlyComponentManager.create (http://localhost:7357/assets/vendor.js:14316:31)
                at Object.evaluate (http://localhost:7357/assets/vendor.js:57289:25)
                at AppendOpcodes.evaluate (http://localhost:7357/assets/vendor.js:55594:19)
                at LowLevelVM.evaluateSyscall (http://localhost:7357/assets/vendor.js:59095:22)
                at LowLevelVM.evaluateInner (http://localhost:7357/assets/vendor.js:59051:14)
                at LowLevelVM.evaluateOuter (http://localhost:7357/assets/vendor.js:59043:14)
        message: >
            global failure: Error: Assertion Failed: Attempting to inject an unknown injection: 'service:test-hftnb-embroider-2@my-service'
        negative: >
            false
        browser log: |
            {"type":"error","text":"\n\nError occurred:\n\n- While rendering:\n  -top-level\n    application\n      index\n        demo-component\n\n"}
            {"type":"error","text":"\n\nError occurred:\n\n\n\n"}
    ...
@NullVoxPopuli
Copy link
Collaborator

can you provide more information?

  • ember-cli version
  • embroider versions
  • yarn/npm why @embroider/*
  • stack trace
  • etc?

@angelayanpan
Copy link
Collaborator Author

Thanks! I'm working on it. just forked the repo from #1072

@angelayanpan angelayanpan changed the title HFTNB syntax and service injection service injection can't be found in Embroider Optimized Apr 18, 2022
@Windvis
Copy link
Collaborator

Windvis commented Apr 18, 2022

That error message comes from ember-source itself and is potentially triggered when the service isn't registered in the container?

Do you have an "app re-export" for the service in question? I experimented with "addon namespaced" service injections before and ran into similar issues. The service was stripped out of the build because of the staticAddonTrees flag which is enabled in the "optimized" configuration. Since Embroider doesn't analyse service injections yet it doesn't know that a service is used and strips it out of the build if there is no re-export for it in the app/services/service-name.js folder.

So if there is no app re-export, the easiest fix would be to create it again. An other alternative could be to register the service in the container under that namespaced name, but it's more work to setup.

@angelayanpan
Copy link
Collaborator Author

I'm in an addon. where would the app re-export live? under tests/dummy/app ?

@Windvis
Copy link
Collaborator

Windvis commented Apr 18, 2022

In the top level app folder of the addon. So if your service is located in addon/services/some-service.js the re-export would be at app/services/some-service.js.

@ef4
Copy link
Contributor

ef4 commented Apr 18, 2022

This is the expected behavior if you're trying to use services directly out of an addon via the resolver's "@" syntax. Since that could target literally any file in any addon, supporting it means we couldn't optimize anything.

The issue is specifically with staticAddonTrees: true, because that setting declares that everything out of all your addons' module namespaces will be consumed via imports.

This is the line that's not supported if you want to set staticAddonTrees to true:

https://github.com/angelayanpan/test-hftnb-embroider-2/blob/7f8123874111f80c81e0d8db271e877c45b8d9e8/tests/dummy/app/components/demo-component.js#L6

It's equivalent to reexport the service from the app tree as @Windvis suggests. If the reason you aren't reexporting is to avoid potential naming collisions, you can follow a pattern very similar to what you have now:

- @service('test-hftnb-embroider-2@my-service') myService;
+ @service('test-hftnb-embroider-2-my-service') myService;

This would corresponding with the file `app/services/test-hftnb-embroider-2-my-service.js` in the addon.

@angelayanpan
Copy link
Collaborator Author

what if the addon doesn't have app folder?

@ef4
Copy link
Contributor

ef4 commented Apr 18, 2022

You can create one. As long as it's a normal v1 addon and doesn't customize treeForApp in index.js, it will use an app folder that you create.

@elwayman02
Copy link
Contributor

We'd prefer not to create an app folder in our addons. That's not a solution that scales very well - every service created in every addon would have to re-export the service to a new name. We can already inject services with the proper namespacing, so what can be done to continue using the existing ergonomics without creating unnecessary re-export requirements?

@NullVoxPopuli
Copy link
Collaborator

some options:

  • you could create a lint that auto-updates your app re-exports for you
  • convert to v2 addon (which creates the "app re-exports" for you, in package.json)
  • help iterate on stuff like: Explicit Service Injection emberjs/rfcs#502

@ef4
Copy link
Contributor

ef4 commented Apr 19, 2022

@NullVoxPopuli your first two suggestions don't help with their particular scaling problem. The problem is that doubling the number of service modules in the build is expensive in a huge app. It doesn't matter whether they're auto generated or not.

@elwayman02 the other alternative is to take direct responsibility for importing and registering the services, preferably in the files that consume them. Done by hand, that looks something like:

import Component from '@ember/component';
import { inject as service } from '@ember/service';
import { action } from '@ember/object';
+ import MyService from 'test-hftnkb-embroider-2/services/my-service';
+ window.define('test-hftnkb-embroider-2/services/my-service', function() { return MyService });

export default class DemoComponent extends Component {
  @service('test-hftnb-embroider-2@my-service') myService;

  @action
  clickedOnSomething() {
    this.myService.empty();
  }
}

It would be possible to implement a staticServices flag analogous to staticComponents that would emit the above pattern automatically. To safely use such a flag, you'd need to eliminate all non-string-literal arguments to the service decorator across your app and all addons, and eliminate all runtime use of owner.lookup("service:something").

That would actually let us drop all of app/services from being eagerly included, trusting instead to the direct import-and-register pattern.

It's also probably possible to swap out the implementation of the service decorator to avoid the window.define, effectively polyfilling something like the explicit service injection RFC.

@elwayman02
Copy link
Contributor

In the short-term, we're going to try utilizing package metadata as a workaround for each addon to inform Embroider of its implicit-modules.

@angelayanpan
Copy link
Collaborator Author

want to provide a follow-up before we close this:
we now have a custom adapter where services are included in the build as implicit modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants