Node-like module resolution #909
Conversation
ark120202
force-pushed
the
node-module-resolution
branch
4 times, most recently
from
3fc08f9 to
dd66b88
Compare
ark120202
force-pushed
the
node-module-resolution
branch
from
dd66b88 to
260c71d
Compare
| "ts-node": "^8.6.2" | ||
| }, | ||
| "peerDependencies": { | ||
| "ts-node": "*" |
There was a problem hiding this comment.
Why do we need this as peer dependency? isn't the regular dev dependency enough?
There was a problem hiding this comment.
It's an optional peer dependency. It is required for Yarn 2, since it doesn't allow importing unreferenced packages
| { | ||
| name: "mode", | ||
| description: | ||
| 'In "lib" mode, imported modules are emitted without being resolved. Visit https://typescripttolua.github.io/docs/TODO for more details.', |
There was a problem hiding this comment.
TODO: Make sure documentation link leads somewhere.
| import { Chunk } from "."; | ||
| import { CompilerOptions } from "../../CompilerOptions"; | ||
|
|
||
| export function chunkToAssets(chunk: Chunk, options: CompilerOptions) { |
There was a problem hiding this comment.
Not sure about the 'Assets' name (also for the file name), it's too generic. What does this mean? Maybe we can come up with a more descriptive name.
There was a problem hiding this comment.
I've tried to match webpack terminology wherever applicable (https://github.com/webpack/webpack/blob/1936615458352f38ff517d258b936d690a7b631f/lib/Compilation.js#L175-L196)
| return ts.createProgram(Object.keys(input), options, compilerHost); | ||
| } | ||
|
|
||
| export interface TranspiledFile { |
There was a problem hiding this comment.
These aren't generic utils, but utils for managed-api (they are in a separate file because we import them in tests). TranspiledFile is the result of createEmitOutputCollector defined below
|
|
||
| const srcFilePath = path.resolve(__dirname, "errors", "error.ts"); | ||
| const outFilePath = path.resolve(__dirname, "errors", "error.lua"); | ||
| const srcFilePath = resolveFixture("errors/error.ts"); |
There was a problem hiding this comment.
Are these paths normalized to work on windows too? Thoughts about making resolveFixture (...parts: string[]) => path.resolve(__dirname, "__fixtures__", ...parts);?
There was a problem hiding this comment.
Generally TypeScript normalizes user-supplied paths to /, so mixed ones are fine too. I don't mind making these platform-specifc if you'd like though
|
|
||
| test("resolve relative module paths", () => { | ||
| const result = loadConfigImport("test", "test", resolveFixture("load-config-import"), "./ts.ts"); | ||
| expect(result).toMatchObject({ result: true }); |
There was a problem hiding this comment.
Is this really the only thing we can assert here?
There was a problem hiding this comment.
Do you have any suggestions? loadConfigImport returns whatever is exported by specified file
|
Would love to see this merged, what is the status of this PR? |
We still want this functionality, but this PR is stale and not ready. This will most likely not be merged, but parts of it will end up in other changes as we add this functionality. |
3 participants
Closes #432.
within. For example, now "./foo" may resolve to "./foo.ts" and "./foo/index.ts".rootDirResolution that goes intonode_modulesout of project root is not implemented yet.@noResolveannotation, or should imported with non-esm syntax (i.e. usingrequire)Transpileroutput.