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

Rewire API added to module twice #57

Closed
mbfisher opened this issue Sep 15, 2015 · 4 comments
Closed

Rewire API added to module twice #57

mbfisher opened this issue Sep 15, 2015 · 4 comments

Comments

@mbfisher
Copy link
Contributor

A contrived example:

MyClass.js

export default class MyClass {
}
index.js

const MyClass = require('./MyClass');
MyClass.foo = 'bar';

export default MyClass;

This leads to an error whereby __Rewire__ is added to the MyClass export in MyClass.js and index.js, causing Object.defineProperty to fail the second time around:

Object.defineProperty(_defaultExport, '__Rewire__', {
    'value': __Rewire__,
    'enumberable': false
});

I'd love have a go at a PR for this but have never worked with the Babel internals and it looks a bit mad...

M

@TheSavior
Copy link
Contributor

Interesting. I'd imagine this could be a problem with any of the rewire flavors. I recently ran into this error with rewire-global but didn't look into it yet. Any module that just exports one of its dependencies exports would have this problem.

@i-like-robots, have you come across this? @jhnns, perhaps we should be adding a check that these functions don't already exist in https://github.com/jhnns/rewire/blob/master/lib/getDefinePropertySrc.js

This is further reason why we should figure out how to be sharing all this code between all these modules ala i-like-robots/rewireify#17

@mbfisher
Copy link
Contributor Author

I have a commit which fixes my instance of this. I've been able to test the changes in exit but not in ExportDefaultDeclaration. I've never worked with the Babel API before - is it along the right lines?

M

@speedskater
Copy link
Owner

@mbfisher thanks for reporting this issue and creating the PR. Your issue is resolved by #62.

Generally patterns like in your sample were an existing class is changed and then reexported should be avoided, as this always modifies the original class. If this is intended and you want to be able to rewire the modified as well as the original module it is recommended to use the RewireApi object.

@mbfisher
Copy link
Contributor Author

mbfisher commented Oct 1, 2015

Cheers!

M

On Thursday, 1 October 2015, speedskater notifications@github.com wrote:

@mbfisher https://github.com/mbfisher thanks for reporting this issue
and creating the PR. Your issue is resolved by #62
#62.

Generally patterns like in your sample were an existing class is changed
and then reexported should be avoided, as this always modifies the original
class. If this is intended and you want to be able to rewire the modified
as well as the original module it is recommended to use the RewireApi
object.


Reply to this email directly or view it on GitHub
#57 (comment)
.

Mike Fisher

Email: mike@mbfisher.com
Skype: mb_fisher
GitHub: mbfisher

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

No branches or pull requests

3 participants