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

Change the key used for imported files #2

Closed
kentcdodds opened this issue Sep 25, 2017 · 4 comments · Fixed by #3
Closed

Change the key used for imported files #2

kentcdodds opened this issue Sep 25, 2017 · 4 comments · Fixed by #3

Comments

@kentcdodds
Copy link
Owner

  • import-all.macro version: latest
  • node version: latest
  • npm (or yarn) version: latest

Relevant code or config:

import importAll from 'import-all.macro'
const files = importAll.sync('./some-files/*/index.js')

Let's assume that some-files is a directory full of other directories each with an index.js. What's going to happen is it'll produce this:

import _index from './some-files/a/index.js'
import _index2 from './some-files/b/index.js'

const files = {index: _index, index: _index2}

Suggested solution:

I'm thinking that the property name should be changed from the filename to the relative path. So it'd be:

const files = {'./some-files/a/index.js': _index, './some-files/b/index.js': _index2}

I'd love to hear what folks think. cc @threepointone.

This would be a breaking change. I honestly foresaw this, but thought I could avoid it, but I've already bumped into it being annoying so 🤷‍♂️

@fatfisz
Copy link
Collaborator

fatfisz commented Sep 27, 2017

Just had this problem, was very surprised :/

Wouldn't keeping the keys as paths relative to opts.filename of the processed file be ok? What is the rationale for the current behavior of shortening the keys?

@kentcdodds
Copy link
Owner Author

What is the rationale for the current behavior of shortening the keys?

It was just me being dumb. I'll fix it up in a few hours. Unless you want to 😉

@fatfisz
Copy link
Collaborator

fatfisz commented Sep 27, 2017

Sadly I've no time for this now, so it'd be awesome if you could :)

@fatfisz
Copy link
Collaborator

fatfisz commented Sep 27, 2017

Actually nvm, I'm preparing the fix right now!

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 a pull request may close this issue.

2 participants