-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Add compile() method to complement process() for transformers #7018
Comments
I don't fully understand what you're asking for here. Do you want a hook into the runtime? What exactly is the current API missing? Do you need to execute some code instead of working on source code (text only)? Also, do you have an API in mind? |
Currently, at module construction, Jest will detect and run the transformer, here. This allows Currently, a transformer can have the following properties/methods
I'm asking for another method that a transformer could provide that is called as a interceptor to the call. This would allow decorating the |
So we invoke some function ( |
The I imagine a transformer.complete(wrapper,
localModule,
filename,
this._environment.global,
this._createJestObjectFor(
filename,
require: LocalModuleRequire
)
) This would provide access to the wrapper function and many of the vars needed to execute it. In the example above I provided a subset of args since the others can be gotten by way of the ones provided (but you could provide the exact args for flexibility down the line). The |
Thanks for the example! I don't think it makes sense to stick this on API-wise I like your suggestion. Only thing I would change is to pass an object instead of 5 arguments - I find it easier to use when I don't have to check the docs or implementation for positional arguments (although good IDEs mitigate this). @thymikee @rickhanlonii @mattphillips thoughts on this? |
No clever thoughts on that, but it sounds like it would be a part of transforming pipeline, as it hooks into transformer results. And it could ship as a single transform, instead of asking users to set 2 separate options. |
I agree with @thymikee it would be nice to not ask for 2 separate options if possible. I don't know much about this part of Jest's internals so not sure I can make a great recommendation on this but whatever we go with I'm happy to help and learn more :) |
Ya, that's cool too. In my own unrelated code I use a wrapper pattern which is As @thymikee mentions the method could end up being another property on the result of |
It's not a transform though. How would I be able to use JSX with I don't know anything about how esm works (does it insert runtime checks? work on AST? extract some parts and execute it?), so maybe I'm misunderstanding. Use case for a working babel transform, but still the goodness of esm: https://twitter.com/dan_abramov/status/935160018508369920 |
Maybe a shift if terms is needed. Instead of transform it could be seen as a module construction pipeline with hooks to process source code, wrapper arguments, etc.
In those cases you're wanting to use a generic transpiler so likely sticking with
The By exposing a Adding another hook doesn't invalidate or break Babel. If the word |
Could you post how (ish) you'd implement the transformer? Both |
@SimenB Sure! function process(source, filename) {
const { code } = ESMCachingCompiler.compile(source, { filename, ...})
return {
code,
complete(wrapper, args) {
const [mod] = args
ESMRuntime.enable(mod, {})
return Reflect.apply(wrapper, args)
}
} |
Ah, that's pretty neat! @cpojer @aaronabramov do you have any thoughts on the API here? I would still like for this to be possible to use together with e.g. Babel, but if this is an OK first go we should do it. We could also go crazy and have an |
I think that would be more of allowing multiple transformers per pattern match. A new
I'd like a more generic approach if possible, instead of a fixed/hard-coded approach. Jest handles the entire module construction so some hook into it, like a |
I'm not convinced adding a feature to transformers solely to support one type of transformer is a good idea. |
Hooks to various phases of the Jest module construction and execution can be generally useful without being specific or hardcoded for a single transformer. |
Do we have 3-5 other real use cases for a hook like that? |
The loaders I'm aware of are There are also packages that tap into
and other CJS module methods which may or may not work with Jest since Jest constructs modules in a way that doesn't leverage the traditional module creation/execution process. Update: Without a hook here is the way I've got to tackle this... const hooked = { __proto__: null }
makeRequireFunction.process = (content, filename, { testEnvironment }) => {
const EVAL_RESULT_VARIABLE = "Object.<anonymous>"
const Environment = require(testEnvironment)
const { runScript } = Environment.prototype
if (! hooked[testEnvironment]) {
hooked[testEnvironment] = true
Environment.prototype.runScript = function (...args) {
const result = Reflect.apply(runScript, this, args)
const wrapper = result[EVAL_RESULT_VARIABLE]
result[EVAL_RESULT_VARIABLE] = function (...args) {
const [mod] = args
/* hook it here */
return Reflect.apply(wrapper, this, args)
}
return result
}
}
/* transform code */
return { code }
} |
Interesting, where would you put that code? |
That code will be-in/or-exposed-in "transform": {
"\\.m?js$": "esm"
} |
Ah, gotcha! That will not work with a docblock setting the env then, as that overrides what's in One possibility might be to create |
That's good to know. I could sniff for the doc comment I guess.
Ya, naw. I don't think I'll be doing that. The workaround for not having an exposed hook is just that, a workaround. I'm not thrilled about it, but until there's something better it's a way to unblock support and let devs get on their way. |
Yeah, definitely! Getting something out would be super awesome indeed. |
@jdalton seeing this #4842 (comment) are you still interested in proposed feature, or maybe transformer is good enough? |
Hi @thymikee! My use of |
Ok, cool! I'm just not sure if we squeeze this in for v24. But since this is non breaking, I guess we can give it some more time. |
This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days. |
This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
🚀 Feature Proposal
Add support for a
compile()
like method to complementprocess()
for transformers.Motivation / Pitch
I'm writing an
esm
loader and trying to make it work as the ESM support layer for Jest (without using Babel). I found I could makeesm
provide aprocess()
method and be used as a"transform": "esm"
which is great! However,esm
anchors its runtime to themodule
object created and ran through thewrapper()
. Normally this isn't a problem, sinceesm
owns the implementation it handles this inModule#compile
orModule#_extensions
. However, Jest constructs modules from scratch so there is no place foresm
to hook into and decorate themodule
object. A hook likecompile
orinit
orbeforeCompile
would be one way of enabling this.Related to #4842 (comment).
The text was updated successfully, but these errors were encountered: