rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[api-extractor] Add handling of file reference directives

Open willmtemple opened this issue 5 years ago • 7 comments
trafficstars

This is a first stab at supporting path-based references like:

/// <reference path="path/to/xyz.d.ts" />

in API Extractor.

Would fix: #1761 (CC @jeremymeng)

We use these references as part of a shimming pattern in the Azure SDK for JavaScript, but we aren't able to use dtsRollup for packages that need them because d.ts rollup currently just doesn't handle them.

Because these references are relative to the SourceFile that they are collected from, the path that is emitted has to be corrected to be relative to the ultimate file. I've written the logic for this as internal functions next to the Collector and DtsRollupGenerator, but could relocate them as needed.

I have not tested this on Windows yet.

willmtemple avatar Jun 22 '20 22:06 willmtemple

Thanks for making a PR! 😁

Could you add a test case to the rushstack\build-tests\api-extractor-scenarios folder? (The test cases get registered in rushstack\build-tests\api-extractor-scenarios\config\build-config.json.)

octogonz avatar Jun 23 '20 10:06 octogonz

@octogonz I am getting a build failure on Windows in node-core-library. blocking api-extractor-scenarios from building.

I'm just building using rush update then rush build in Windows PowerShell, and I've set my execution policy to "Bypass" for the process. I don't know if there are other special instructions for building on windows because I can't find documentation on the build process.

Here is the error:

 FAIL  lib\test\Executable.test.js
  ● Executable.spawnSync("npm-binary-wrapper") edge cases 1

    Error

      86 |     expect(result.error).toBeUndefined();
      87 |     expect(result.stderr).toBeDefined();
    > 88 |     expect(result.stderr.toString()).toEqual('');
      89 |     expect(result.stdout).toBeDefined();
      90 |     const outputLines = result.stdout
      91 |         .toString()
      Error: expect(received).toEqual(expected)

      Expected value to equal:
        ""
      Received:
        "The directory name is invalid.
      "
      at executeNpmBinaryWrapper (lib/test/Executable.test.js:88:38)
      at Object.<anonymous> (lib/test/Executable.test.js:119:12)

  ● Executable.spawnSync("npm-binary-wrapper") edge cases 2

    Error

      86 |     expect(result.error).toBeUndefined();
      87 |     expect(result.stderr).toBeDefined();
    > 88 |     expect(result.stderr.toString()).toEqual('');
      89 |     expect(result.stdout).toBeDefined();
      90 |     const outputLines = result.stdout
      91 |         .toString()
      Error: expect(received).toEqual(expected)

      Expected value to equal:
        ""
      Received:
        "The system cannot find the path specified.
      "
      at executeNpmBinaryWrapper (lib/test/Executable.test.js:88:38)
      at Object.<anonymous> (lib/test/Executable.test.js:129:12)

  ● Executable.spawnSync("npm-binary-wrapper") edge cases 2

    Error

      86 |     expect(result.error).toBeUndefined();
      87 |     expect(result.stderr).toBeDefined();
    > 88 |     expect(result.stderr.toString()).toEqual('');
      89 |     expect(result.stdout).toBeDefined();
      90 |     const outputLines = result.stdout
      91 |         .toString()
      Error: expect(received).toEqual(expected)

      Expected value to equal:
        ""
      Received:
        "The system cannot find the path specified.
      "
      at executeNpmBinaryWrapper (lib/test/Executable.test.js:88:38)
      at Object.<anonymous> (lib/test/Executable.test.js:139:12)

 PASS  lib\test\PackageName.test.js
 PASS  lib\test\Text.test.js
 PASS  lib\test\ProtectableMap.test.js
 PASS  lib\test\PackageJsonLookup.test.js
 PASS  lib\test\Path.test.js
 PASS  lib\test\Sort.test.js
 PASS  lib\test\JsonSchema.test.js
 PASS  lib\test\FileSystem.test.js
[13:20:24] Error - 'jest' sub task errored after 8.75 s
 Jest tests failed
[13:20:24] 'test' errored after 18 s
[13:20:24]
About to exit with code: 1
Process terminated before summary could be written, possible error in async code not continuing!
Trying to exit with exit code 1
The script failed with exit code 1

willmtemple avatar Jun 25 '20 20:06 willmtemple

I am getting a build failure on Windows in node-core-library. blocking api-extractor-scenarios from building.

I'm just building using rush update then rush build in Windows PowerShell, and I've set my execution policy to "Bypass" for the process. I don't know if there are other special instructions for building on windows because I can't find documentation on the build process.

@willmtemple I cannot repro this.

What is the full path of your repo (pwd in PowerShell)? I wonder if one of the parent paths has weird characters, or exceeds MAX_PATH. I typically clone my repo as C:\Git\rushstack to minimize these issues.

octogonz avatar Jun 26 '20 05:06 octogonz

I don't know if there are other special instructions for building on windows because I can't find documentation on the build process.

API Extractor docs are here. General repo instructions are here.

octogonz avatar Jul 08 '20 01:07 octogonz

@willmtemple Following up -- are you still working on this PR? Do you need anything else from us?

octogonz avatar Aug 28 '20 07:08 octogonz

@octogonz Had to put this down for a while. The build issue I was running into was indeed a MAX_PATH issue. This week I'm going to revisit this. Referring to your comment in the linked issue:

For a rollup, there's some question as to how those paths should be handled.

Do you have any thoughts on the two approaches to take here? This implementation is leaving the file references intact in the output bundle, just rewriting them to be relative to the output location, but the other alternative of course is that we could include the referenced file in the rolled-up d.ts.

willmtemple avatar Aug 31 '20 16:08 willmtemple

This PR also seems to fix #3000

Is there anything I can do to help?

pd4d10 avatar Feb 06 '22 13:02 pd4d10