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

Failsafe requirejs unset/reset #169

Merged
merged 1 commit into from Jun 23, 2017
Merged

Failsafe requirejs unset/reset #169

merged 1 commit into from Jun 23, 2017

Conversation

petschki
Copy link
Member

Introduced in 5c75aa8 it might happen,
that there is no define or require defined. handle this gracefully.

Introduced in 5c75aa8 it might happen,
that there is no `define` or `require` defined. handle this gracefully.
@petschki petschki requested a review from thet June 23, 2017 06:39
@coveralls
Copy link

Coverage Status

Coverage remained the same at 74.403% when pulling 18bc1fe on failsafe-requirejs-unset-reset into f65ddfb on 1.x.

@petschki
Copy link
Member Author

Hm ... one more problem of the new unset/reset scripts is, that wildcard.foldercontents resource hack isn't working anymore ... see https://github.com/collective/wildcard.foldercontents/blob/master/wildcard/foldercontents/views.py#L944 ... @thet is this unset/reset thing really needed? I never ran into those Mismatched anonymous define errors ...

@thet
Copy link
Member

thet commented Jun 23, 2017

@petschki - i just got this error with the plone.app.event 2.x javascript, which register itself as requirejs module, if it's available.
It's probably also needed for scritps like datatables.
Unset/reset requirejs is done the same way in Plone 5.

I recommend to fix wildcard.foldercontents.

@@ -1,4 +1,4 @@
var _old_define = define;
Copy link
Member

Choose a reason for hiding this comment

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

did you get an error here?
I wonder, because I'd have expected, if _old_define isn't defined, than define is just set to undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

when in debug mode I get this error:

bildschirmfoto 2017-06-23 um 09 22 09

but when in production mode, the error is gone. how that? strange ...

Copy link
Member

Choose a reason for hiding this comment

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

Alright...

@thet
Copy link
Member

thet commented Jun 23, 2017

LGTM

@thet thet merged commit 82bcd75 into 1.x Jun 23, 2017
@thet thet deleted the failsafe-requirejs-unset-reset branch June 23, 2017 10:02
@thet
Copy link
Member

thet commented Jun 23, 2017

@petschki
Hm. Your code did not work for me.
When require and define aren't defined before they are unset, then the mockup widgets bundle will set them they won't be set to undefined afterwards as _old_define and _old_require are undefined.
But I need require and define to be undefined. I think that it's a rare case that require and define are defined before the mockup script.

I'll come back to this later. But now, it's weekend!

@petschki
Copy link
Member Author

petschki commented Jun 26, 2017

@thet you're right ... if they aren't define, they aren't reset. So the reset-script needs fallback:

    define = typeof(_old_define) !== 'undefined' ? _old_define : undefined;
    require = typeof(_old_require) !== 'undefined' ? _old_require : undefined;

this should do the trick ...

@petschki
Copy link
Member Author

the wildcard.foldercontents hack should be fixed there ... I'll create an issue ...

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.

None yet

3 participants