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

feat: add a default export #18

Closed
wants to merge 1 commit into from

Conversation

laggingreflex
Copy link
Contributor

@laggingreflex laggingreflex commented Nov 28, 2016

This adds a default export object containing all named exports.

Eg.:

// @create-index

export king from './king.js'
export queen from './queen'
export default {
  king,
  queen
}

Fixes #11

Keeps camelCase support from #16

@laggingreflex
Copy link
Contributor Author

laggingreflex commented Nov 28, 2016

Above doesn't work, it actually needed to be more like this. Fixed.

// @create-index

import _king from './king.js';
export const king = _king;

import _queen from './queen';
export const queen = _queen;

export default {
  king,
  queen
}

This adds a default export object containing all named exports.

Eg.:
```js
import _king from './king.js';
export const king = _king;

import _queen from './queen';
export const queen = _queen;

export default {
  king,
  queen
};
```
@laggingreflex
Copy link
Contributor Author

@gajus What are your thoughts on this? #27 build on top of this a lot, rebasing it against this would be a lot different than rebasing it against current master. Let me know if you're happy with this. If so I can actually merge this and #27 together.

@gajus
Copy link
Owner

gajus commented Dec 8, 2016

feat: add a default export

What is the reason for wanting this?

This breaks static analyses, i.e. it invalidates the purpose of using ES modules in the first place. Default exports should be avoided whenever possible.

@laggingreflex
Copy link
Contributor Author

It fixes #11. Copying the relevant part of the problem:

For example

proj
├── king.js
└── queen
    ├── bar.js
    └── foo.js

creates

proj
├── king.js
├── index.js
└── queen
    ├── index.js
    ├── bar.js
    └── foo.js

where queen/index.js is correct (as long as only named exports are imported from it):

// proj/queen/index.js
export bar from './bar.js';
export foo from './foo.js';

but proj/index.js isn't

// proj/index.js
export king from './king.js';
export queen from './queen'; // < because queen has no default export

This adds a default export to queen/index.js

// proj/queen/index.js
import _bar from './bar.js';
export const bar = _bar;

import _foo from './foo.js';
export const foo = _foo;

export default {
  foo,
  bar
}

Without this create-index is incompatible with itself. Recursive (creating) indexing also wouldn't work if it encounters an index created with create-index which will cause the above (queen) problem.

@gajus
Copy link
Owner

gajus commented Dec 8, 2016

If I have a project structure such as:

./
├── king.js
├── index.js
└── queen
    ├── index.js
    ├── bar.js
    └── foo.js

Then I expect to be able:

import {king} from './';
import {bar, foo} from './queen';

Anything else, e.g. import {queen} from './' is invalid use case.

Invalid in a sense that it invalid use of static analyses and possibility of using shaking. Of course, correct me if I am wrong.

My assumptions are based on the knowledge of existing mechanisms used to transpile the code, namely babel-plugin-transform-es2015-modules-commonjs and how webpack and rollup handles dependency resolution.

@laggingreflex
Copy link
Contributor Author

You're right it does promote bad design.

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