FriendsOfFlarum / byobu

Well integrated, advanced private discussions for your Flarum forum.
https://discuss.flarum.org/d/4762-flagrow-by-bu-well-integrated-advanced-private-discussions
MIT License
54 stars 33 forks source link

Export stuff that might be needed elsewhere #112

Closed askvortsov1 closed 4 years ago

askvortsov1 commented 4 years ago

The necessity for this came about while working on https://github.com/FriendsOfFlarum/drafts/pull/10.

I am not sure that this is the correct way to accomplish this.

dsevillamartin commented 4 years ago

You have to use Object.assign for flarum.core.compat

https://github.com/flarum/tags/blob/b2f74cbb9020f474e1659aba6ae3f1e408742216/js/src/forum/index.js#L36-L40

askvortsov1 commented 4 years ago

That's the first thing I tried, but wasnt sure we wanted to do it that way: wouldn't that add it to the flarum namespace vs an fof namespace?

dsevillamartin commented 4 years ago

There's no standard here to uphold. If you want to keep them separate, do not export it as compat - simply export everything in compat itself. We do not need this compat object then.

clarkwinkelmann commented 4 years ago

The core compat object is meant for core extensions only.

I think the approach here is the correct approach, but I don't like the fact the file is named compat when it has nothing to do with the compat feature of Flarum core.

What I have done in my Formulaire extension to export everything while keeping the code tidy is:

index.js:

export * from './components';
export * from './pages';

app.initializers.add('kilowhat-formulaire', () => {
    //
}

components/index.js:

import ErrorPage from './ErrorPage';
import FormGroup from './FormGroup';
import FormTitle from './FormTitle';
import LoadingIndicator from './LoadingIndicator';

export const components = {
    ErrorPage,
    FormGroup,
    FormTitle,
    LoadingIndicator,
};

And repeat with an index.js file in each directory.

But this means the exports look like

{
    components: {
        TheComponent: TheComponent
    }
}

instead of

{
      'components/TheComponent': TheComponent
}

So I guess we can't import it with the @vendor/namespace/component syntax, but that syntax is broken already anyway I think.

askvortsov1 commented 4 years ago

Yeah that syntax is unfortunately broken. It would be nice to figure out how to get extensions to export stuff automatically (if that's even a possibility), but thats not really a priority in the release cycle rn. I'll change it to export an object of components, and an object of helpers?

dsevillamartin commented 4 years ago

It would be nice to figure out how to get extensions to export stuff automatically (if that's even a possibility)

I think I've looked at this, but I either didn't know what to look up or didn't find anything afaik. 😕

I'll change it to export an object of components, and an object of helpers?

Sounds good.

askvortsov1 commented 4 years ago

@datitisev is this ok?

dsevillamartin commented 4 years ago

I think it's better if we do what Clark does in his formulaire extension, instead of having a whole file dedicated to it (like core does).

askvortsov1 commented 4 years ago

My thought was avoiding clogging index.js with ALL the imports.

dsevillamartin commented 4 years ago

Well, that's not what Clark said. Clark has a components/index.js - we can do the same. Have components/index.js and helpers/index.js (can be required as ./components and ./helpers), and export * from them.

askvortsov1 commented 4 years ago

Ah my bad. I'll have this fixed up shortly

askvortsov1 commented 4 years ago

@datitisev done, sorry for the delay