Skip to content

Conversation

@Stereobit
Copy link
Contributor

Hey,

I've been running into some problems when wrapping require JS define and require functions. I often return functions that have to be called as constructor functions. My first approach was checking if the function is called as a constructor function and if so calling the wrapped function with the new keyword as well. Which is impossible as long as you want variable arguments.

The next approach was using a ES6 Object.setPrototypeOf() polyfill. It's working but it is strongly discouraged, because it is very slow and unavoidably slows down subsequent execution in modern JavaScript implementations.

So I ended up adding a option to prevent wrapping the functions arguments.

~ Tobi

@mattrobenolt
Copy link
Contributor

Can you give me an example where this fails so I can figure out a better way to potentially do it automatically?

mattrobenolt added a commit that referenced this pull request Jan 19, 2014
add option to prevent wrapping also the functions arguments
@mattrobenolt mattrobenolt merged commit 9419e48 into getsentry:master Jan 19, 2014
@Stereobit
Copy link
Contributor Author

Sure. Checkout http://jsfiddle.net/k7uaT/

You can add/remove raven to see the problem.

@mattrobenolt
Copy link
Contributor

Ohh, I see. JavaScript sucks. :(

Thanks for the PR.

kamilogorek pushed a commit that referenced this pull request Jun 12, 2018
mydea added a commit that referenced this pull request May 12, 2025
See
https://github.com/nodejs/import-in-the-middle/releases/tag/import-in-the-middle-v1.13.1

### Bug Fixes

* handling of circular dependencies
([#181](nodejs/import-in-the-middle#181))
([b58092e](nodejs/import-in-the-middle@b58092e))
* importing JSON files
([#182](nodejs/import-in-the-middle#182))
([8c52014](nodejs/import-in-the-middle@8c52014))
* warning from use of context.importAssertions
([#179](nodejs/import-in-the-middle#179))
([8e56cf1](nodejs/import-in-the-middle@8e56cf1))

This version is already allowed by our range, but in order to ensure
everybody gets this, bumping it here.
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