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

Cleanup element-mixin leftovers from modulizer #5330

Merged
merged 1 commit into from
Aug 17, 2018

Conversation

TimvdLippe
Copy link
Contributor

This was some leftover output from modulizer that we can remove.

Reference Issue

Closes #5315

Copy link
Member

@kevinpschaaf kevinpschaaf left a comment

Choose a reason for hiding this comment

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

LGTM

@kevinpschaaf kevinpschaaf merged commit 568840b into master Aug 17, 2018
@kevinpschaaf kevinpschaaf deleted the TimvdLippe-patch-1 branch August 17, 2018 17:28
@@ -760,7 +749,7 @@ function _regLog(prototype) {
*/
export function register(prototype) {
registrations.push(prototype);
undefined && _regLog(prototype);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is right. In 2.x this only logged if Polymer.log was set. Modulizer made it never log, which isn't great, but after this change we'll always log, which we also don't want.

IMO we should invent a global name that, when truthy, means that we'll log here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rictic Ah crap, you are correct! Reinvent the Polymer.log setting? 😂

Copy link
Member

Choose a reason for hiding this comment

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

I'd say let's just remove the registration logging altogether in 3.x. I don't know that this feature was ever documented, and the whole registration tracking system is fairly dubious now that WC V1 has a registry API.

Will make a new PR to remove that line; dumpRegistrations can remain.

Copy link
Member

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

4 participants