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

Re-enable api extractor when building abort-controller #17986

Merged
2 commits merged into from
Oct 5, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sdk/core/abort-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"build:samples": "echo Obsolete",
"build:test": "tsc -p . && rollup -c 2>&1",
"build:types": "downlevel-dts types/src types/3.1",
"build": "npm run clean && tsc -p . && rollup -c 2>&1 && npm run build:types",
"build": "npm run clean && npm run extract-api && rollup -c 2>&1 && npm run build:types",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need tsc -p . I believe.

The files section can now be updated to have the bundled .d.ts files

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremymeng the extract-api script already runs tsc.

I agree on updating the files list. It includes all files under types in addition to a shim file, so I would suggest to keep the shim and remove types entries. Furthermore, export everything from the d.ts file created by the api extractor from the shim (see here for example). Actually since downlevel types is being used, you will need to have two such shims, one for < v3.6 and another for everything else (see here for example).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremymeng the extract-api script already runs tsc.

Ah that's right! This is running the npm script, not the api-extractor command. On a side note, our build scripts are not consistent in this. I will see if we have an issue tracking that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some work on this for core packages in #16248. This change does not align with the standardization I did. @ramya-rao-a could you consider aligning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template package has the below

npm run clean && tsc -p . && rollup -c 2>&1 && api-extractor run --local && npm run build:types

Is this what you are referring to @deyaaeldeen ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deyaaeldeen, @jeremymeng Can either of you create an issue to track the files ask?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"check-format": "prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"clean": "rimraf dist dist-* temp types *.tgz *.log",
"docs": "typedoc --excludePrivate --excludeNotExported --excludeExternals --stripInternal --mode file --out ./dist/docs ./src",
Expand Down