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

SCRIPTS: Allow importing from an index file by using a directory name #1593

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Snarling
Copy link
Collaborator

I was revising some of my own scripts when I noticed that importing from an index file (by just importing from the directory name) was not yet supported. This should add support for that.

@d0sboots
Copy link
Collaborator

d0sboots commented Aug 17, 2024

What's the usecase here? (Edit: vs just adding /index in your code)

@d0sboots
Copy link
Collaborator

(Adding a bit more flavor): Module import semantics are (intentionally) underspecified in the various specs, it's up to the embedder to decide what to do with it. So, it's literally up to us, and of course Node has made this landscape very complicated with a number of mostly-compatible but subtly different module resolution schemes.

As a result, I'm somewhat leery of supporting "just import this directory" (especially since "directories" don't even work entirely correctly for us), unless there's a really good reason for it.

@Snarling
Copy link
Collaborator Author

It's not providing a new function that can't be accomplished by writing out /index, but it's nicer to not have to write that out, especially if you are used to not having to write it out in your other development environment.

My own specific use case is just that I wanted a shorter import statement for using box.js - instead of importing from "box/box", I would prefer to just be able to import from "box", so I thought to use this feature but it didn't work. But my own use case isn't really the best reason to do a change like this.

The more compelling reason for a change like this would be that most (to my knowledge) other development environments do allow this (including ours - a usage we have within our own codebase is Themes/data + all subfolders). Some developers like to use barrel files a lot, and they are probably used to being able to name the barrel file index and then omit the filename from the import statement. It's a worse experience if they want to use it like they're used to being able to and then have to change it because we don't support that usage.

@d0sboots
Copy link
Collaborator

I don't think I've ever seen anyone asking for this feature or wondering about its non-existence, but I also don't hang out in help-scripts. Ultimately, I don't have anything against this aside from the fact that it makes module resolution (already the trickiest part of our code) more complicated.

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

Successfully merging this pull request may close these issues.

2 participants