Skip to content
New issue

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

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

Already on GitHub? # to your account

[FEATURE] Implement injection parameter normalization RFC. #17858

Merged
merged 2 commits into from
May 9, 2019

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Apr 4, 2019

Implements emberjs/rfcs#451, and fixes the error message so it won't throw if properties aren't passed (new EmberObject())

@pzuraq pzuraq force-pushed the feat/classic-class-owner-tunnel branch from 791b80b to 8007793 Compare April 4, 2019 20:01
@rwjblue rwjblue added the Octane label Apr 11, 2019
@rwjblue rwjblue force-pushed the feat/classic-class-owner-tunnel branch from 8007793 to 7f112e0 Compare May 3, 2019 00:53
@rwjblue rwjblue changed the title [FEATURE beta] Passes the owner through to classic classes [FEATURE] Implement injection parameter normalization RFC. May 3, 2019
@rwjblue
Copy link
Member

rwjblue commented May 3, 2019

Just updated to more closely match emberjs/rfcs#451, should be ready for review.

@rwjblue rwjblue requested a review from krisselden May 3, 2019 00:54
Copy link
Contributor

@chadhietala chadhietala left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me. Just a small nit.

@@ -40,13 +45,15 @@ const factoryMap = new WeakMap();

const prototypeMixinMap = new WeakMap();

let PASSED_FROM_CREATE;
let initCalled; // only used in debug builds to enable the proxy trap
const initCalled = DEBUG ? new WeakSet() : undefined; // only used in debug builds to enable the proxy trap
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you are going to fail IE11 builds due to the usage of WeakSet.

Copy link
Member

Choose a reason for hiding this comment

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

@chadhietala - WeakSet is imported from @ember/polyfills (which uses a WeakMap with a boolean value when an actual WeakSet is not present).

@rwjblue rwjblue requested a review from chadhietala May 8, 2019 18:50
@rwjblue rwjblue merged commit 1731b14 into master May 9, 2019
@rwjblue rwjblue deleted the feat/classic-class-owner-tunnel branch May 9, 2019 16:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants