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

Feature/improvements #8

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

patric-eberle
Copy link

This PR tries to reduce the amount of fallback merges and conditions. This PR should not break or extend any functionality.

  • src/bem-cn/bem-names.js
    • The configuration fallback was removed since it will always be available from the method caller (as long as kept inside plugin scope).
    • The conditions were rearenged to reduce number of evaluations.
  • src/bem-cn/index.js
    • The configuration fallback was removed since it will always be available from the method caller (as long as kept inside plugin scope).
    • The conditions were rearenged to reduce number of evaluations.
  • src/globals.js
    • The fallback config has been moved to it's own file. Initially this was because it was needed by multiple methods. In the meantime it's only purpose is to also be usable for the tests.
  • src/utils.js
  • Some of the conditions were removed since they are covered by others. (e.g. val can not be null anymore if val && typeof val === 'Object' where true already.
  • src/vue-plugin.js
    • Uses now external fallback config.
    • Reuse of isString method.
  • The tests had to be adjusted so they always deliver the now needed configuration.

I also tested this with our current project.

@c01nd01r
Copy link
Owner

c01nd01r commented Oct 1, 2018

Nice improvements, Patric! Thank you.
I will try to see this PR in more detail in the near time.

…efined, moves namespaces and block concatination to mixin init (adjusts tests therefore).
@patric-eberle
Copy link
Author

Hmmm... forgot to add the hyphenate to the block definition. Guess there is no test for this ;) Will have to fix that.

@patric-eberle
Copy link
Author

I added the hyphenate conversion for the block now and extended the tests.

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

Successfully merging this pull request may close these issues.

2 participants