Skip to content

Add some test and comments for module.js #1113

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

Merged

Conversation

lanzhiheng
Copy link
Contributor

No description provided.

@lanzhiheng lanzhiheng force-pushed the feature/add-tests-for-module-js-with-some-comments branch from ea20825 to c9058f5 Compare January 2, 2018 08:38
Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

Thank you for improving the comment and test code! I've commented a few things.


module.addChild('v1', new Module({}))
module.addChild('v2', new Module({}))
module.removeChild('v2')
Copy link
Member

Choose a reason for hiding this comment

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

We should also check the _children just before calling removeChild to confirm whether it actually works or not.

expect(module._rawModule).toEqual(originObject)

module.update(newObject)
expect(module._rawModule).not.toEqual(newObject)
Copy link
Member

Choose a reason for hiding this comment

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

I think this assertion is not needed because it does not matter whether the whole object strictly equals newObject or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ktsn Thank you for your comment, I will fix it today.

@lanzhiheng lanzhiheng changed the title Add some test and comments for module.js [WIP] Add some test and comments for module.js Jan 3, 2018
@lanzhiheng lanzhiheng changed the title [WIP] Add some test and comments for module.js Add some test and comments for module.js Jan 4, 2018
Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

Thanks!

@ktsn ktsn merged commit e3ae423 into vuejs:dev Jan 4, 2018
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

Successfully merging this pull request may close these issues.

2 participants