-
-
Notifications
You must be signed in to change notification settings - Fork 535
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
Avoid double-transformation of a file when multiple ts-node hooks are registered #409
Comments
There's no great way to do this except for throwing an error and making it impossible for your code to execute. If I silently allow one and not the other, especially in this case, it's pretty likely the compiler options or configuration options are different and that'll result in an inconsistent runtime. I'd recommend you use a different module (there should be no reason for a config module to alter your runtime, it should do transpilation separately) or make an issue on that module. I'll keep this open to remind me to throw an error if you try to double register in the future. This will not solve your problem though. |
I face similar issue with "config" module. It registers ts-node explicitly https://github.com/lorenwest/node-config/blob/02c98b7f5947ece93357652b69d9cb3da6decfd3/lib/config.js#L878 @blakeembrey, after i read your comment it is clear that fix should be done on "config" side. |
@sixmen node-config/node-config#458 should fix your issue |
I face similar issue with webpack-cli.
I modify the config file path with ts-node. But webpack-cli re-register ts-node. If there is a way to detect whether it is registered, the caller can solve this problem. |
@ZSkycat Do you have a suggestion for how this could be detected? My only thought is storing a global that can be accessed somehow by all modules, but I have no consensus on where this should be stored. FWIW, you should look at using environment variables in that case too - it's why they're provided. |
@blakeembrey So, I use it in a strange way. |
@ZSkycat It looks like two issues:
|
I have a suggestion for how this could be detected export function isRegistered(){
return require.extensions['.ts'] != undefined || require.extensions['.tsx'] != undefined;
} Maybe I should create PR? |
That’s how this module works but it doesn’t tell you if it’s already been registered - anything could be on those extensions already. A global would be the most appropriate way to solve this, but not sure where it should live. |
I think, whether it is "ts-node" or anything else. They register the extension '.ts' or '.tsx'. "ts-node" re-register will cause problems see here https://github.com/TypeStrong/ts-node/blob/master/src/index.ts#L412
function registerExtension (
ext: string,
ignore: RegExp[],
register: Register,
originalHandler: (m: NodeModule, filename: string) => any
) {
const old = require.extensions[ext] || originalHandler
let handler = function (m: any, filename) {
if (shouldIgnore(filename, ignore)) {
return old(m, filename)
}
const _compile = m._compile
m._compile = function (code: string, fileName: string) {
debug('module._compile', fileName)
return _compile.call(this, register.compile(code, fileName), fileName)
}
return old(m, filename)
};
handler.__tsNode = true;
handler.__context = {};
require.extensions[ext] = handler;
} export function isRegistered(){
return require.extensions['.ts'] != undefined || require.extensions['.tsx'] != undefined;
}
export function isTsNodeRegistered(){
if (require.extensions['.ts'] != undefined && require.extensions['.ts'].__tsNode) return true;
if (require.extensions['.tsx'] != undefined && require.extensions['.tsx'].__tsNode) return true;
return false;
} |
@ZSkycat Your assumptions are incorrect here. Just because someone has registered This is why I suggested it needs to be a global of some kind. It could be within the module, such as just |
I spent all day figuring out why my code was compiled twice and found that karma re-registers require('ts-node').register(/* your options */)
// karma re-registers ts-node
// https://github.com/karma-runner/karma/blob/7617279f68507642fdc7eb0083916f3c6c2a28cb/lib/config.js#L37
// override with noop so it's only called once per process
require('ts-node').register = function() {} |
@gmathieu Sorry about that. It's a tricky problem, can you turn it off in karma? When the error gets added to avoid re-registering, it might cause some trouble for you if not. |
@blakeembrey karma's config parser automatically registers |
How about attached to the
I suggest this because it seems the "module" module is responsible for creating |
There’s actually already a global symbol now that could be used. The reason I haven’t done this issue is because it’s a little more complex - you need to check for overlaps in registration directories and not just for a second register. We could put all the registrations in an array and iterate over it when a new registration occurs, check if the directory is already registered. This sort of issue is also less common now too. I haven’t seen the confusion come up as often, but another solution could be to add a log to the compiler errors when it looks like a TS to TS issue. |
@d3lm moving discussion here, and renaming the issue to more accurately describe what I believe is the best solution. Highlighting Blake's comment above:
What if we devise a way to detect when another ts-node instance has already transformed a given file, and we perform this check on a per-file basis? This avoids the need for any complex logic to check for overlapping Here is one possible solution:
|
I have labelled this issue to indicate we will accept a pull request implementing the proposal outlined in my previous comment. Please do not hesitate to ask me for guidance. Once you know where to look, the implementation and tests should be straightforward. My limited time is needed elsewhere, so I won't be able to implement this any time soon. |
@cspotcode Your proposed solution makes sense to me 👍 |
Maintainer edit: If you would like to submit a pull request implementing this feature, I have described a straightforward implementation here: #409 (comment)
If I require 'ts-node/register' twice, it does not run well. For examples,
my.ts:
index.js:
If I run with
node index.js
, it works. But if I run withts-node index.js
, it fails.In the real world, I run app with ts-node.
And config module, which I use, also register ts-node ( https://github.com/lorenwest/node-config/blob/master/lib/config.js#L872 ).
The text was updated successfully, but these errors were encountered: