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

collapse top level index folder #151

Open
benawad opened this issue Aug 14, 2020 · 10 comments
Open

collapse top level index folder #151

benawad opened this issue Aug 14, 2020 · 10 comments
Labels
enhancement New feature or request

Comments

@benawad
Copy link
Owner

benawad commented Aug 14, 2020

its weird to have a folder called index so just bring up all the files in it one level

@AnatoleLucet
Copy link
Collaborator

From this:

src/
├── index
│   ├── header
│   │   └── button.ts
│   └── header.ts
└── index.ts

to this:

src/
├── header
│   └── button.ts
├── header.ts
└── index.ts

?

I mean, why not. But I think we must implement this under a config option / cli flag and not make it the default.

@AnatoleLucet AnatoleLucet added the enhancement New feature or request label Aug 14, 2020
@benawad
Copy link
Owner Author

benawad commented Aug 14, 2020

But I think we must implement this under a config option / cli flag and not make it the default.

How come?

@AnatoleLucet
Copy link
Collaborator

AnatoleLucet commented Aug 14, 2020

In a single entry point folder (like our src folder on Destiny's codebase) why not, but I'd be the chaos on a multiple entry points folder. For example, when using Destiny on sub folders of src this approach may not fit since each folder have multiple entry points (e.g. a components folder).

@AnatoleLucet
Copy link
Collaborator

AnatoleLucet commented Aug 14, 2020

I think Destiny should be able to run on any kind of js / js-like codebase without any issue and then be configurable to suit the user's preferences

@benawad
Copy link
Owner Author

benawad commented Aug 15, 2020

How does it break down for multiple entry points?

@AnatoleLucet
Copy link
Collaborator

Current output:

components/
├── header
│   ├── wrapper
│   │   └── uniqueButton.js
│   └── wrapper.js
├── footer
│   ├── links
│   │   └── link.js
│   └── links.js
├── header.js
└── footer.js

Your proposal output:

components/
├── wrapper
│   └── uniqueButton.js
├── links
│   └── link.js
├── wrapper.js
├── links.js
├── header.js
└── footer.js

How can you find out which file is dependent on whom?
Also, what if both footer.js and header.js has a deps with the same name e.g. layout.js (not a shared deps, just the same file name). We'd need to rename these two files so they can be in the same folder.

@benawad
Copy link
Owner Author

benawad commented Aug 16, 2020

What do you think of collapsing it only if the folders name is index?

@AnatoleLucet
Copy link
Collaborator

I'm not sure. Some apps name their entry point main.js, app.js, server.js, etc... I don't know if it's a good idea to do something specific based on a file's name (except for ignoring folders / files like node_modules or something).
I still think we should do this as a flag / config option.
We may need something like a destiny init to ask this kind of questions though.

@AnatoleLucet
Copy link
Collaborator

AnatoleLucet commented Aug 17, 2020

Or else, we can detect if there's one single file at top level. It seems pretty obvious but it didn't click until now...

src/
├── [name]
└── [name].*

Then why not. But with this, I think we should also give an option to the user so he can disable this feature if he wants to.

But I'm not sure how we'll deal with shared deps. Shall we do this only if there's no shared folder at top level, or merge it with the one that will be moved to top?
If we merge them, what about duplicated files name? Shall we throw a warning that we can't bring everything to top level because two files have the same name? Or maybe we can rename one of the two?

@benawad
Copy link
Owner Author

benawad commented Aug 18, 2020

that's a good point, I think merging is probably the right choice, but I'll think on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants