Skip to content

Change exports usage#230

Merged
ljharb merged 1 commit intoljharb:masterfrom
btd:patch-1
Oct 14, 2017
Merged

Change exports usage#230
ljharb merged 1 commit intoljharb:masterfrom
btd:patch-1

Conversation

@btd
Copy link
Contributor

@btd btd commented Oct 13, 2017

Hello.

free exports usage (when it is not assignmnet for really exporting something) confusing rollup to wrap such modules in additional wrappers.

This PR remove exports for self referencing exporting symbols.

Thanks.

What do you think about replacing build with rollup (i mean dist/qs.js) - it will produce smaller file, and much readable/debuggable? (i can prepare PR)

Copy link
Owner

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to use rollup; it violates the spec. Similarly, if it can't understand exports. syntax - a CJS standard for years - that's a pretty massive flaw with it.

That said, using exports is definitely a bad practice anyways, and this is a good change. Thanks!

@ljharb ljharb merged commit 6f0586f into ljharb:master Oct 14, 2017
@btd
Copy link
Contributor Author

btd commented Oct 14, 2017

Yep, i understand what you mean. Thank you for merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants