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

chore: Use fully qualified paths #28

Merged
merged 1 commit into from
May 3, 2024
Merged

chore: Use fully qualified paths #28

merged 1 commit into from
May 3, 2024

Conversation

arv
Copy link
Collaborator

@arv arv commented May 3, 2024

This is mostly to unblock @cloudflare/vitest-worker-pool which has a bug that prevents it from resolving dir/a/b to dir/a/b/index.js.

cloudflare/workers-sdk#5749 (comment)

This is mostly to unblock `@cloudflare/vitest-worker-pool` which has a
bug that prevents it from resolving `dir/a/b` to `dir/a/b/index.js`.

cloudflare/workers-sdk#5749 (comment)
@kibae
Copy link
Owner

kibae commented May 3, 2024

Hi, @arv
It seems inappropriate to enforce the expression of artificially importing .js files in a project developed in TypeScript.
What problem are you trying to solve?

@kibae
Copy link
Owner

kibae commented May 3, 2024

Is there no way to solve the issue by modifying the tsconfig.json to enable the use of fully qualified paths in .js files within an npm package, rather than in TypeScript source code?

@arv
Copy link
Collaborator Author

arv commented May 3, 2024

I agree that it is a unfortunate. TS does not change module specifiers. It has no options to rewrite them.

If you change package.json to use "type": "module" it is enforced in TS as well which prevents these kind of errors to introduced but then again it changes the npm package so it would be better to be a pure esm package #30.

@kibae
Copy link
Owner

kibae commented May 3, 2024

This file is the index.js file included in the npm package. How about using webpack to bundle it into a single file to avoid any issues with require? Although tree shaking won't be possible, I think it won't be a problem since the code for this project is small.

"use strict";
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
    if (k2 === undefined) k2 = k;
    var desc = Object.getOwnPropertyDescriptor(m, k);
    if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
      desc = { enumerable: true, get: function() { return m[k]; } };
    }
    Object.defineProperty(o, k2, desc);
}) : (function(o, m, k, k2) {
    if (k2 === undefined) k2 = k;
    o[k2] = m[k];
}));
var __exportStar = (this && this.__exportStar) || function(m, exports) {
    for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p);
};
Object.defineProperty(exports, "__esModule", { value: true });
__exportStar(require("./logical-replication-service"), exports);
__exportStar(require("./output-plugins/test_decoding/test-decoding-plugin"), exports);
__exportStar(require("./output-plugins/pgoutput"), exports);
__exportStar(require("./output-plugins/wal2json/wal2json-plugin"), exports);
__exportStar(require("./output-plugins/wal2json/wal2json-plugin-options.type"), exports);
__exportStar(require("./output-plugins/wal2json/wal2json-plugin-output.type"), exports);
__exportStar(require("./output-plugins/decoderbufs/decoderbufs-plugin"), exports);
__exportStar(require("./output-plugins/decoderbufs/decoderbufs-plugin-output.type"), exports);

@kibae
Copy link
Owner

kibae commented May 3, 2024

@arv I just read through https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping#node-16 and tried applying it. I think the changes you made are correct. I'm planning to merge this PR, and I would like to invite you to this project. 🤗

@kibae
Copy link
Owner

kibae commented May 3, 2024

@arv Let me know your npmjs.org ID, and I'll give you permissions to publish to npm as well.

@kibae kibae merged commit 34b9013 into kibae:main May 3, 2024
5 checks passed
@arv
Copy link
Collaborator Author

arv commented May 3, 2024

My npm username is arv

https://www.npmjs.com/~arv

@arv arv deleted the qualify-paths branch May 3, 2024 13:12
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