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

ContainerDebugAdapter.catalogEntriesByType returns objects of wrong type #120

Closed
gabrielgrant opened this issue Oct 28, 2015 · 3 comments
Closed

Comments

@gabrielgrant
Copy link

The ContainerDebugAdapter's catalogEntriesByType() method matches objects by just testing whether the type name appears anywhere within the class name. The result is that many false-positives are returned.

eg for model:

var debugAdapter = appInstance.lookup('container-debug-adapter:main');
console.log(debugAdapter.catalogEntriesByType('model'))

returns:

["ember-data/model", "ember-data/serializers/active-model", "ember-data/adapters/active-model", "ember-cli-test-model-waiter/instance-initializers/test-model-waiter", "ember-cli-test-model-waiter", "hub/ember-cli-test-model-waiter/tests/modules/embe…er/instance-initializers/test-model-waiter.jshint", "hub/instance-initializers/test-model-waiter", "action", "operation", "user", "action.jshint", "operation.jshint", "user.jshint", "action-test", "action-test.jshint", "operation-test", "operation-test.jshint", "user-test", "user-test.jshint", __ember_meta__: Meta]

when I'm pretty sure it should just be:

["action", "operation", "user"]

Not sure how this interacts with pods layouts and the comment in there about different prefixes, but to match "normal" layouts, it seems it should be execing against something like RegExp('^[a-zA-Z0-9-_]+\/' + type + '\/(.*)$')

Note: I originally filed this against ember-cli (for some reason) as ember-cli/ember-cli#5002

@teddyzeenny
Copy link
Contributor

Yes. A regexp would be more accurate. Some things to note though. In pods the module name will end with the type so we should modify the regexp to also accept modules that end with the type name (like myapp/company/model.js). I also don't think we can rely on always having only one word before the type.

The inspector currently uses class detection (DS.Model.detect) after getting the list to filter out false positives.

@gabrielgrant
Copy link
Author

Ah, interesting. I dug into the inspector code a little while trying to sort this out, but didn't catch that.

When you say you don't think we can rely on there only being one word before the type, do you mean something like myapp/models/supertype/subtype.js or myapp/multi-named-pod/model.js or myapp/my-pod/sub-pod/model.js or something else entirely?

Assuming both those pods layouts are valid, I think RegExp('^[a-zA-Z0-9-_]+\/(.*)\/' + type + '$') should work. I'd probably be inclined to keep the detection of pods and non-pods separate, just to avoid descending too far into the depths of regex hell.

@stefanpenner
Copy link
Contributor

as of 3.0 this is now provided by ember (correctly)

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

3 participants