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

[Vue] utility function for creating angularjs adapters #18146

Merged
merged 30 commits into from Oct 15, 2021

Conversation

diosmosis
Copy link
Member

Description:

The adapter code is pretty similar for most directives, so I created a utility function automatically define the angularjs adapter based on some configuration (as also suggested by @peterhashair). It works and in the long run can keep the asset size from getting too much bigger, and will potentially also cut down on time to migrate directives. But it's rather complex (mostly for directives like piwik-dialog that don't use an isolate scope), so I'm not sure how good an idea it is. @tsteur @sgiehl (or anyone else) do you have any opinions?

image

Review

…ectly watch all plugin files, fix path issue in webpack externals, add vue matomo dialog use example to ExampleVue
… the chunk is not stored in the same directory as the merged asset JS)
@diosmosis diosmosis added the Needs Review PRs that need a code review label Oct 13, 2021
@diosmosis diosmosis added this to the 4.6.0 milestone Oct 13, 2021
@tsteur
Copy link
Member

tsteur commented Oct 13, 2021

Looks generally good @diosmosis . It only requires some angular test fixes as they are failing now.

@diosmosis
Copy link
Member Author

@tsteur this PR is based off of #18127, just to double check that code was reviewed as well, correct?

@tsteur
Copy link
Member

tsteur commented Oct 14, 2021

@diosmosis yes that included the other PR as I only looked and tested this PR but not the other one specifically (there was no needs-review label)

@diosmosis
Copy link
Member Author

@tsteur 👍 thanks for the reply, I closed the other PR and will merge this when the build passes.

@diosmosis diosmosis merged commit 2f80606 into 4.x-dev Oct 15, 2021
@diosmosis diosmosis deleted the vue-adapter-utility branch October 15, 2021 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants