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

Missing identifier for export. This member will not be included in the barrel #8

Open
diegovilar opened this issue Mar 10, 2024 · 9 comments

Comments

@diegovilar
Copy link

diegovilar commented Mar 10, 2024

Trying the just published version compatible with TS5, but I'm gettin warnings in existing barrel files. For what I can tell, it happens on the first widlcard export in the barrel file (but existing barrel files should make the plugin skip creating or inspecting one, right?)

image

@zaguiini
Copy link
Owner

Hey Diego, thanks for reporting.

That behavior sounds about right at first thought. You could create a virtual barrel file of existing barrel files, and in that case, the existing barrel file would not have any of its members included in the barrel because of the missing identifier.

One possibility would be to include an option in the plugin configuration that accepts a glob pattern so we can know which paths to ignore. But I'm unsure how big of a problem that is. It's a warning, not an error.

@diegovilar
Copy link
Author

diegovilar commented Mar 11, 2024

Not sure if I understood correctly, so forgive me if I got it all wrong, but in the case of existing barrel files, the plugin doesn't create a virtual one, right? Or does it? I'm confused about that, because having a warning generated by the plugin makes me think it is doing something with the existing barrel file.

And you're right, it's just a warning in the end of the day. But the thing about warnings that can't really be addressed is that they make us too comfortable ignoring warnings...

If it is NOT creating a virtual barrel in case of an existing one, then maybe the warning is unecessary all together. What do you think about that? That would also allow for the introduction of the plugin in existing projects that already have lots of barrel files without generating tons of warnings.

If it IS creating a virtual barrel on top of the existing one, wouldn't it be better if the plugin completely skipped the creation of a virtual one? That would support the case where we actually want to create a barrel manually so we can cherry pick what is exported from the directory.

@zaguiini
Copy link
Owner

I'm confused about that, because having a warning generated by the plugin makes me think it is doing something with the existing barrel file.

That makes sense.

then maybe the warning is unecessary all together

I disagree, the warning is necessary because the person might assume that a default function with no identifier might be picked up, but it won't.

If it is NOT creating a virtual barrel in case of an existing one

AFAIK (It's been 8 months since I wrote this code), it is not creating a virtual barrel for existing files. As I said above, it is creating a barrel for the barrel files in the directory, not for nested folders.

That would also allow for the introduction of the plugin in existing projects that already have lots of barrel files without generating tons of warnings.

This is a fair point. I think one option would be to let the user suppress the warning instead. Through the glob matcher idea for example.

@diegovilar
Copy link
Author

🤔

Looking at the screenshot below, knowing that the warning in the existing barrel file points to the wildcard export of the CEP.ts file, which contains only named exports as shown on the top panel, I'm completely lost about what is the warning telling me, to be honest.

Is it because the existing barrel itself exports only wildcards?

image

@zaguiini
Copy link
Owner

zaguiini commented Mar 17, 2024

@diegovilar, here's how TVB works:

  • we scan the folder for typescript files
  • we get the named exports (export const, export function, export class) for the files
  • we reexport the members by their identifier (const identifier, function identifier, class identifier)

In the case of a wildcard export (export *), we don't have an identifier, so there's no way to include it in the barrel file. That's why you see the warning in existing barrel files, and you'd see the same warning if you tried to do export default function() {} because there would be no identifier for TVB to rely on.

That's why I'm talking about adding an option, or a comment at the top of the file, ignoring specific files so that the warning goes away.

The only other option I could think of would be to ignore barrel files, but that would be misleading if you consider the following tree:

- components/
-- buttons/
---- index.ts
---- primary.tsx
---- secondary.tsx
-- inputs/
---- index.ts
---- text.tsx
---- numeric.tsx

Both components/**/index.ts, which already exist, have the same structure:

export * from './primary'
export * from './secondary'

Now, if I wanted to import something with TVB using the following import statement:

import { PrimaryButton } from './components'

That would not work since there are no identifiers specified in the existing barrel files, therefore they would not be included in the generated barrel file. That's why we can't remove the warning from existing barrel files.

One could argue that we could move away from exporting the members by identifier and instead export everything from the file, but I like the control we have by exporting individual members. The existing approach would also let us ignore some members from being picked by TVB once #6 lands, so I'm not sure about the refactor.

@diegovilar
Copy link
Author

diegovilar commented Mar 17, 2024

One could argue that we could move away from exporting the members by identifier and instead export everything from the file, but I like the control we have by exporting individual members. The existing approach would also let us ignore some members from being picked by TVB once #6 lands, so I'm not sure about the refactor.

When we create barrel files, we usually do it to export everything from the location and shorten import statements, right? That's the most frequent reason to do so. If one needs control over what is exported, one usually cherry-picks in the barrel file what to export. That's how we do it normally, right? Recursively.

When I think of a plugin that automates the creation of barrel files, It seems natural to me to expect it to behave as close as possible (if not exactly) to how manually created barrel files would work:

  1. It would automatically create a virtual barrel file in every directory and in each virtually created barrel file it would export * from... all files and subdirectories in that directory.
  2. If a real barrel file is found at some level, instead of exporting directly from other files or directories in that level, it would export * from... that existing barrel file. Here we would have control of what to export in said lavel, if we so desire.

No need to introduce control comments to filter anything. The plugin would just do what is natural (export everything) but give us control back where needed by honoring existing barrels.

@zaguiini
Copy link
Owner

That makes sense to me, @diegovilar. Wanna go ahead and create a PR with the changes? I have little to no bandwidth right now.

@diegovilar
Copy link
Author

Cool! I was already hacking my way through it, just didn't want to start a new project to go in another direction when you've already done so much. I'll get back to you when I have something working to see what you think 🤘🏻

@PhilStar101
Copy link

@diegovilar Hi! Did you make it?

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

No branches or pull requests

3 participants