Skip to content

[Vue] Allow passing multiple contexts #461

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

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/Vue/Resources/assets/dist/register_controller.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
function registerVueControllerComponents(context) {
function registerVueControllerComponents(contexts) {
const vueControllers = {};
const importAllVueComponents = (r) => {
r.keys().forEach((key) => (vueControllers[key] = r(key).default));
};
importAllVueComponents(context);

[].concat(contexts).forEach((context) => importAllVueComponents(context));

window.resolveVueComponent = (name) => {
const component = vueControllers[`./${name}.vue`];
const component = Object.values(
Object.fromEntries(
Object.entries(vueControllers).filter(([key]) => key.endsWith(`${name}.vue`)))
)[0];

if (typeof component === 'undefined') {
throw new Error('Vue controller "' + name + '" does not exist');
}
Expand Down
9 changes: 6 additions & 3 deletions src/Vue/Resources/assets/src/register_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,21 @@

'use strict';

export function registerVueControllerComponents(context: __WebpackModuleApi.RequireContext) {
export function registerVueControllerComponents(contexts: any) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we:

Suggested change
export function registerVueControllerComponents(contexts: any) {
export function registerVueControllerComponents(context: __WebpackModuleApi.RequireContext|Array< __WebpackModuleApi.RequireContext >) {

const vueControllers: { [key: string]: object } = {};

const importAllVueComponents = (r: __WebpackModuleApi.RequireContext) => {
r.keys().forEach((key) => (vueControllers[key] = r(key).default));
};

importAllVueComponents(context);
[].concat(contexts).forEach((context: __WebpackModuleApi.RequireContext) => importAllVueComponents(context));

// Expose a global Vue loader to allow rendering from the Stimulus controller
(window as any).resolveVueComponent = (name: string): object => {
const component = vueControllers[`./${name}.vue`];
const component = Object.values(
Object.fromEntries(Object.entries(vueControllers).filter(([key]) => key.endsWith(`${name}.vue`)))
)[0];
Copy link
Member

Choose a reason for hiding this comment

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

tbh, this makes my head spin a little bit.

If I import 2 contexts, and each contains a Hello.vue, the 2nd one will "win" over the first, correct?

With the new code, is the "key" inside vueControllers now a longer path (e.g. common/Hello.vue instead of just Hello.vue)? What makes these code changes needed?

Copy link
Author

@MantraMediaMike MantraMediaMike Sep 20, 2022

Choose a reason for hiding this comment

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

This would then not be possible or just very hacky with the way require context works.

I personally think that a project should never have 2 components with the same name and that the component names should always be very explicit and never as generic as just Menu. (https://vuejs.org/style-guide/rules-essential.html)

But if this should be possible, I think we can close the pull request for now?


if (typeof component === 'undefined') {
throw new Error(`Vue controller "${name}" does not exist`);
}
Expand Down
3 changes: 3 additions & 0 deletions src/Vue/Resources/assets/test/fixtures/entry/Entry.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<h1>This entry is called {{ entry }}</h1>
</template>
20 changes: 18 additions & 2 deletions src/Vue/Resources/assets/test/register_controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,32 @@

import {registerVueControllerComponents} from '../src/register_controller';
import {createRequireContextPolyfill} from './util/require_context_poylfill';
import Hello from './fixtures/Hello.vue'
import Hello from './fixtures/common/Hello.vue'
import Entry from './fixtures/entry/Entry.vue'

require.context = createRequireContextPolyfill(__dirname);

describe('registerVueControllerComponents', () => {
describe('registerSingleVueControllerComponentContext', () => {
it('test', () => {
registerVueControllerComponents(require.context('./fixtures', true, /\.vue$/));
const resolveComponent = (window as any).resolveVueComponent;

expect(resolveComponent).not.toBeUndefined();
expect(resolveComponent('Hello')).toBe(Hello);
expect(resolveComponent('Entry')).toBe(Entry);
});
});

describe('registerMultipleVueControllerComponentsContexts', () => {
it('test', () => {
registerVueControllerComponents([
require.context('./fixtures/common', true, /\.vue$/),
require.context('./fixtures/entry', true, /\.vue$/)
]);
const resolveComponent = (window as any).resolveVueComponent;

expect(resolveComponent).not.toBeUndefined();
expect(resolveComponent('Hello')).toBe(Hello);
expect(resolveComponent('Entry')).toBe(Entry);
});
});