-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Attempting to define property on object that is not extensible #28
Comments
@0x80 I think the error is regression of making the export properties not enumerable. I would suggest to try to use babel-plugin-rewire version 0.1.12 in the mean time. If this does not fix your error could please provide a usage sample as a PR ? |
@speedskater with that version I'm getting a different error in my tests: And if I use the app code in a browser with the plugin on the loader I see I'm going to be on a holliday in about two hours and I'm afraid I won't be able to provide a good sample as a PR before then... |
Okay. Thank you I will try to fix the problem asap. But it will take at least till tomorrow. So have a nice holliday and hopefully the bug is fixed afterwards :) |
👍 |
I ran into this problem when building rewire-global because of modules that were returning things like booleans which you can obviously not add properties to. The fix that worked for me was to selectively expose the rewire functionality on modules that made sense to rewire at all: https://github.com/TheSavior/rewire-global/blob/master/index.js#L11 |
@TheSavior thanks for your suggestion. I think the problem should be solved by converting the primitive types to real objects. see: https://javascriptweblog.wordpress.com/2010/09/27/the-secret-life-of-javascript-primitives/ Will tackle the problem today evening. |
It feels super sketchy to convert people's primitives to objects. And as the article mentions, that won't work for booleans. With the basic rewire library, not every module has the Using IMHO, It is better to only add the rewire functions to modules the user might potentially want to rewire instead of modifying potential logic and behavior (converting the primitives to objects) in every module. |
I am just preparing. It is based on http://www.2ality.com/2011/04/javascript-converting-any-value-to.html. Thanks for your hint regarding boolean values. For boolean values I will use the normal assignment with standard properties. It has the drawback that enumerable properties are exported in this case but at least it should work. Although I agree with you that changing only the code which is actually rewired would be nicer i think it is currently not possible. I think to achieve this a major rewrite would be required. |
I guess I am making the claim that nobody ever wants to rewire modules that export primitives. So only adding the rewire methods on things that export |
@TheSavior I have just run into another issue and i think you are definitely right. Will publish the fix in some hours :). |
@TheSavior, @0x80 The Problem should be fixed in the latest version 0.1.14. Only default exports which are of type function or object will get the rewire properties attached. Could you please let me know if the fix works for you. |
That’s still issue for me. I’m trying to use it with karma and before I’m able to do anything I get
My tests.webpack.js is dumb and simple:
Version of plugin: Am I missing something? UPD: I tried to switch to Chrome and get new error:
|
I placed a debugger in |
I think this issue has to be reopened. We have the similar problem with Uncaught TypeError: __$Getters__[name] is not a function |
@fobdy Thanks for reopening the issue. Could you please test it with 0.1.23-beta-3 ? |
I ran into the same problem today -
Please let me know if there's anything I can do to assist in debugging! |
@jorispz thanks for testing. It would be great if you could create a PR with a sample, which reconstructs your problem. I will then try to fix it as soon as possible. |
Here is example of issue with Please reopen the issue (now it is in closed status) |
Thanks for providing a sample. Is it possible for you to reduce the test case down to the smallest part that reproduces the error? And it doesn't look like your repo has a test suite that we can run to reproduce the error. If you find that setting up that repo for this is too difficult, you might want to consider adding a fixture to this repo and adding a failing test here for us to fix. |
I created a minimal testcase here: https://github.com/jorispz/babel-plugin-rewire-issue-28 As explained in the README, the problem can be reproduced by running 'npm run test', and disappears when not using the rewire plugin. The cause seems to be the use of Object.assign - see README for details. I hope this helps, let me know if I can help further! |
+1 |
Sorry for the Delay on the latest issues but i am currently in h |
on holidays and will return on the 23rd of november. I will then fix this bug and create a new beta. |
@jorispz nice test case! I just stepped through that in Firefox and noticed something pretty interesting. Here's some code within Redux: function assertReducerSanity(reducers) {
Object.keys(reducers).forEach(function (key) {
var reducer = reducers[key];
var initialState = reducer(undefined, { type: _createStore.ActionTypes.INIT });
if (typeof initialState === 'undefined') {
throw new Error('Reducer "' + key + '" returned undefined during initialization. ' + 'If the state passed to the reducer is undefined, you must ' + 'explicitly return the initial state. The initial state may ' + 'not be undefined.');
}
var type = '@@redux/PROBE_UNKNOWN_ACTION_' + Math.random().toString(36).substring(7).split('').join('.');
if (typeof reducer(undefined, { type: type }) === 'undefined') {
throw new Error('Reducer "' + key + '" returned undefined when probed with a random type. ' + ('Don\'t try to handle ' + _createStore.ActionTypes.INIT + ' or other actions in "redux/*" ') + 'namespace. They are considered private. Instead, you must return the ' + 'current state for any unknown actions, unless it is undefined, ' + 'in which case you must return the initial state, regardless of the ' + 'action type. The initial state may not be undefined.');
}
});
} The |
Here's a snippet from t.blockStatement([
addNonEnumerableProperty(t, defaultExportVariableId, '__Rewire__', universalAccessors['__Rewire__']),
addNonEnumerableProperty(t, defaultExportVariableId, '__set__', universalAccessors['__Rewire__']),
addNonEnumerableProperty(t, defaultExportVariableId, '__ResetDependency__', universalAccessors['__ResetDependency__']),
addNonEnumerableProperty(t, defaultExportVariableId, '__GetDependency__', universalAccessors['__GetDependency__']),
addNonEnumerableProperty(t, defaultExportVariableId, '__get__', universalAccessors['__GetDependency__']),
addNonEnumerableProperty(t, defaultExportVariableId, '__RewireAPI__', universalAccessors['__RewireAPI__'])
])
|
Nevermind about that. But I'm thinking potentially this has something to do with it: exports.hideNotification = _hideNotificationOrig;
exports.__GetDependency__ = _GetDependency__;
exports.__get__ = _GetDependency__;
exports.__Rewire__ = _Rewire__;
exports.__set__ = _Rewire__;
exports.__ResetDependency__ = _ResetDependency__;
exports.__RewireAPI__ = _RewireAPI__;
exports['default'] = _RewireAPI__; so problem from OP was maybe... |
As far as i have seen the problem in your PR is |
@0x80 @kibin @jorispz @CrazyUmka @TheSavior @Dygerati @adamdicarlo Please all have a look at the latest beta 1.0.0-beta-4 . Could you please let me know if this solves your issues. |
@speedskater sorry to hear that it can't be fixed, but I'm fine with closing this issue. Thanks for the help! |
I can confirm that in 1.0.0-beta-4 problem seems disappeared. |
@speedskater don't worry about it - we refactored things around a bit to work around this issue, so don't fix it on my behalf (though feel free to do so regardless :-) |
By now we fixed wildcard imports as well. so we will close this bug. Keep in mind that having multiple wildcard exports in the same file is currently not supported by the plugin. |
How is that not in big bold letters in the README? |
I'm now trying to work with a Karma Webpack setup, and without even trying to mock anything I get the following error from my tests:
Any idea what might be wrong? I'm using babel 5.8 and webpack 1.10
I've set the webpack config to debug: 'inline-source-maps' but the line number from the error is not referencing the original souce. It's outside of the scope of this issue but if someone can give me advise on how to fix that I'd appreciate it :)
The text was updated successfully, but these errors were encountered: