Conversation
microsoft-github-policy-service
bot
added
the
compiler:core
|
All changed packages have been documented.
|
…/microsoft/typespec into fix-self-module-resolution-issue
MaryGao
changed the title
MaryGao
changed the title
self type module resolution issue
MaryGao
requested review from
allenjzhang,
bterlson,
markcowl and
timotheeguerin
as code owners
|
You can try these changes here
|
| return undefined; | ||
| } | ||
| if (pkg.name !== name) continue; | ||
| return loadPackage(dir, pkg); |
There was a problem hiding this comment.
I think the way it is done now is the correct way according the the node resolution spec, you only lookup to the first parent package.json for resolving self. What is the use case?
There was a problem hiding this comment.
The case happened in our cadl-ranch test where during generation our emitter dist code is in the same folder with tsp code and emitter is not released yet.
timotheeguerin
requested review from
JoshLove-msft,
XiaofeiCao,
haolingdong-msft,
iscai-msft,
m-nash,
msyyc,
srnagar,
tadelesh and
weidongxu-microsoft
as code owners
|
API change check API changes are not detected in this pull request. |
|
We'll discuss later on this should have been an allowed feature or not, took your fix and made it in the correct branch #4697 |
|
I agree with @timotheeguerin on the change. We should follow the convention of how a node module is resolved. In csharp emitter, we fixed the build error by specifying the absolute path: |
|
Correction. To follow node module convention, we need to fix our pipeline as well. Because in some regen steps, the emitter module is downloaded as build artifact, and we need to know the path to it: https://dev.azure.com/azure-sdk/public/_build/results?buildId=4226197&view=logs&j=ad8522ef-ffaf-59d1-5ee5-3e85b44db816&t=237d07e5-d6e4-537a-487e-2d10838f2a6b |
4 participants
See comment. In
resolveSelfwe should traverse all possible dirs to find potential module, not directly skip continuing loop when find one package.json file.