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.

Sign up for GitHub

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

Jump to bottom

Added configurable options #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added configurable options #7

wants to merge 1 commit into from

Conversation

nandi95
Copy link
Contributor

@nandi95 nandi95 commented Jan 25, 2021

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    feature
  • What is the current behavior? (You can also link to an open issue here)
    you can't customize accessor and directives
  • What is the new behavior (if this is a feature change)?
    Added option to customise accessor and directives
  • Other information:
    closes Configurable naming for global accessor/directives #2

@nandi95 nandi95 self-assigned this Jan 25, 2021
@nandi95 nandi95 requested a review from fullstackfool January 25, 2021 23:01
@nandi95 nandi95 added the enhancement New feature or request label Jan 25, 2021
nandi95
nandi95 commented Jan 25, 2021
View reviewed changes
rollup.config.js
@@ -21,9 +21,6 @@ export default {
external: [
...Object.keys(pkg.dependencies || {})
],
alias: {
'@/': path.resolve(__dirname, '/src/')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ts takes care of this

nandi95
nandi95 commented Jan 25, 2021
View reviewed changes
rollup.config.js

export default {
input: 'src/index.ts',
output: [
{
file: pkg.main,
format: 'cjs',
sourcemap: true
sourcemap: true,
exports: 'default'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rollup advised in the terminal so added itdocs

nandi95
nandi95 commented Jan 25, 2021
View reviewed changes
tests/vue/ACL.test.ts
expect(wrapper.vm.$acl).toBeInstanceOf(ACL);
});

it('should be able to set the directive names from the config', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this needs to be tested for the other 3 directives as well

@nandi95
Copy link
Contributor Author

nandi95 commented Jan 25, 2021

There are also suggestions in there.
Test is passing on its own. jsdom might have to be reset before the test somehow. (perhaps replace the global document with new JSDOM('<!DOCTYPE html><head /><body /></html>'))?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fullstackfool fullstackfool Awaiting requested review from fullstackfool

At least 1 approving review is required to merge this pull request.

Assignees

@nandi95 nandi95

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Configurable naming for global accessor/directives
1 participant
@nandi95