-
Notifications
You must be signed in to change notification settings - Fork 27
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
Recursive imports #11
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR! However from merging #10, which adds globs, some conflicts have occurred. Could you resolve these? |
I definitely can... I'm at a Code Retreat for most of the day, so I'll tackle it tonight. |
42771d4
to
90bb004
Compare
@vihanb did the rebase and @drgould's test as well as my tests seem to be working fine now. Something to note, the deep recursive imports will also not work with file globbing right now. But root import file globbing still works. When I find some more time in the future I want to go through and refactor a bit to make sure everything is working in tandem. I'll write up some issues for each of them. |
@vihanb just dropping a quick reminder for a PR review in case you missed it. :) |
oh my >_< I had totally forgotten about this PR. I'll update NPM and merge tomorrow |
README.md
Outdated
Items.A = _wcImport; | ||
import _wcImport1 from "./dir/c"; | ||
Items.C = _wcImport1; | ||
const Items.Nested = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this just be Items.Nested = {}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so actually.
Is this going to be released? Would love to use this feature in my project. |
I want to add another comment to reiterate just how useful this PR would be. @vihanb are you still maintaining this project?? |
Yes I will be looking into merging this... hopefully I’ll have time this weekend but they are a few quirks I’ll have to check. I might rewrite parts of this to decouple the logic. |
That’s awesome, thanks! I tried to just use the forked repo via npm but it wasn’t working at compile time, so it would be awesome if it makes its way into the official package. |
Any chance to get this into the project? :) |
A must have 👍 |
is this repo still maintained? |
@vihanb So I've got the initial work on recursive imports working. I'm having a hard time figuring out the best way to handle filters, but normal wildcard works just fine. I tried to avoid re-factoring too much to avoid making things unfamiliar. This is actually my first non-company open source pull request so please forgive anything I might have missed.
If you need anything changed or clarified, then please let me know.
A couple asides:
nodemon
to setup a watching test-runner so I could easily build out this systemyarn
so I've added myyarn.lock
file so I can remove it if need beindex.js
yet, it will be imported withindex
as its key instead of being loaded as the default and extended past that point