### Packages impacted by this PR
@azure/identity-broker
@azure/identity
### Issues associated with this PR
#31338
### Describe the problem that is addressed by this PR
Migrates identity and identity-broker to ESM, tshy, and Vitest
For packages that have been migrated to ESM, Rolled up dts files are no
longer used but they are still packaged because our `files` field
include an entry of `dist/`. Since they are not used anymore, this PR
stops generating them, instead of more complicated approach of excluding
them from `npm pack`.
After moving to ESM/tshy, we no longer build tests/samples. This PR
enables running "typecheck" as an optional rush bulk command, and fixes
some of the errors found.
The known errors reported on files from vite/vitest/chai are ignored.
The update-snippets dev-tool command throws error of reading undefined
in some cases, adding a null check fixes it.
Snippets are updated as well after snippets.spec.ts files are fixed.
- add dep to `uglify-js` in dev-tool
- rush update
- replace "uglifyjs" with "dev-tool run vendored yglifyjs"
- remove dependency to "uglifyjs" in sdk packages
***NO_CI***
- apply lint:fix results
- revert fixes that are not related to consistent-type-imports and cases error
- regenerate .api.md files
- format
- update core-comparison diff
Node made a breaking change in a security release for 18/20 where `spawn()` no longer loads `.cmd` files by default. nodejs/node#52554.
This PR sets the `shell: true` option when running vendored command on Windows.
### Packages impacted by this PR
@azure/attestation
### Issues associated with this PR
#31338
### Describe the problem that is addressed by this PR
Migrates `@azure/attestation` to ESM and tshy.
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
- replace version string with ~8.10.0
- rush update
- [monitor-opentelemetry-exporter] fixing linting error of unnecessary `undefine` type for the optional parameter
### Packages impacted by this PR
@azure/dev-tool
### Issues associated with this PR
Resolves#31325
### Describe the problem that is addressed by this PR
In order to automate as much as possible, we're introducing a set of
codemods to dev-tool's migrate-package command which can be added to by anyone.
These codemods will run in sequence and commit their work in stages for ease
of reviewing
### Packages impacted by this PR
@azure/dev-tool
### Issues associated with this PR
Resolves#29672
### Describe the problem that is addressed by this PR
With the move to ESM and tshy we've been running into issues using the
existing TS_NODE
based loader in dev-tool. Specifically we're seeing errors such as:
```
[run-samples] Error: Must use import to load ES Module: D:\a_work\1\s\sdk\template\template\samples-dev\getConfigurationSetting.ts
require() of ES modules is not supported.
require() of D:\a_work\1\s\sdk\template\template\samples-dev\getConfigurationSetting.ts from D:\a_work\1\s\common\tools\dev-tool\src\commands\samples\run.ts is an ES module file as it is a .ts file whose nearest parent package.json contains "type": "module" which defines all .ts files in that package scope as ES modules.
Instead change the requiring code to use import(), or remove "type": "module" from D:\a_work\1\s\sdk\template\template\package.json.
```
ts-node is being phased out for tsx, a modern alternative that handles
hybrid scenarios like us in much simpler manner.
This PR switches our loader from ts-node to tsx. Now we can run samples
again for both cjs and esm!
***NO_CI***
Currently the rule only handles the case of
```ts
clientMethod(options: MethodOptions) {}
```
but not when the options bag parameter has a default value
```ts
clientMethod(options: MethodOptions = {}) {}
```
This change adds code to handle the latter.
- Add two unit tests
- Update the affected packages to suppress this rule as a warning
- react to the fix
### Packages impacted by this PR
@azure/dev-tool
### Issues associated with this PR
N/A - part of migrations
### Describe the problem that is addressed by this PR
A few quality-of-life improvements as I migrate keyvault packages to
ESM:
1. Allow skipping browser test configuration
2. Ensure linters are happy by appending an empty line to updated json files
3. Replace `tsc -p .` with `dev-tool run build-package` in the scripts section
4. Replace `@azure-tools/test-utils` with `@azure-tools/test-utils-vitest`
5. Remove `tshy` from the dependencies to add since `build-package` uses
a vendored tshy version
### Packages impacted by this PR
dev-tool
### Issues associated with this PR
### Describe the problem that is addressed by this PR
A few QoL additions as we aim to complete the ESM migration:
- output markdown
- Add verbose mode to output individual package status
I only focused on ESM migration as we plan to migrate ESM + Vitest in a
single commit. This all builds off of the work Matt started and allows us to
keep an up-to-date wiki for folks to follow the migration status
This PR fixes an error in extra-api command when a package doesn't
enable the docModel generation, we were trying to open an api.md file
that doesn't exists.
### Packages impacted by this PR
core
### Issues associated with this PR
N/A
### Describe the problem that is addressed by this PR
As we look to migrate all packages to tshy we want to make sure we are
able to move away from tshy if ever needed. This PR adds a build-package command
that can later be used to consolidate some of the commands and decouple a bit
from tshy
### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?
We can just call tshy directly, but one of the "selling" points of tshy
is that we can always eject and do what it does manually if we run into issues.
This helps streamline that without having to update 400 package.json files if
needed.
Frankly, I am not 100% sure if this is needed and I admit that `tshy` is
still an entry in packages' json, but I think we can live with the `tshy`
entry longer than we can with a `tshy` tool migration.
### Packages impacted by this PR
- @azure/dev-tool
- @azure/abort-controller
- @azure/core-amqp
- @azure/core-auth
- @azure/core-client
- @azure-rest/core-client
- @azure/core-http-compat
- @azure/core-lro
- @azure/core-paging
- @azure/core-rest-pipeline
- @azure/core-tracing
- @azure/core-util
- @azure/core-xml
- @azure/logger
- @typespec/ts-http-runtime
### Issues associated with this PR
### Describe the problem that is addressed by this PR
Adds snippets extraction to core.
### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?
### Are there test cases added in this PR? _(If not, why?)_
### Provide a list of related PRs _(if any)_
### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_
### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
This PR creates a merged api.json file from individual api.json files of
subpath exports. The `ApiModel` type from api-extractor-model already
allows multiple `EntryPoint` but so far only one is added.
With this new file, we can also attach more information to the
`metadata` properties. Even though they are not loaded by `ApiModel`, we
could still read that information from the file directly. This saves us
from having to upload multiple files to ApiView.
### Describe the problem that is addressed by this PR
The current overrides for the browser specific files only replace them
if they are under "src" directory. This PR adds the "test" directory for
test coverage
### Packages impacted by this PR
- @azure/dev-tool
### Issues associated with this PR
### Describe the problem that is addressed by this PR
This PR implements a new dev-tool command: `dev-tool run
update-snippets`.
This command looks for code fences in markdown files and JSDoc comments,
and updates them with the contents of test methods in a file named
`snippets.spec.ts`.
For example, the following fence indicates that the contents of a test
named "new_configurationclient" should be used:
````
```js snippet:new_configurationclient
```
````
After running `dev-tool run update-snippets`, the contents of the
snippet will be populated:
````
```js snippet:new_configurationclient
import { ConfigurationClient } from "@azure/template";
import { DefaultAzureCredential } from "@azure/identity";
const client = new ConfigurationClient(
"<app configuration endpoint>",
new DefaultAzureCredential()
);
```
````
To accomplish this, the command uses the TypeScript compiler API to
extract and transpile snippets from `snippets.spec.ts`. Snippets are the
contents of calls to the `it` function. If syntax with the shape
`it(<literal string>, <function with block>)` appears in
`snippets.spec.ts`, it will be considered a snippet that is valid for
injection.
("Function with block" means either a `function () { ... }` expression
or an arrow function with a block on the arrow side (`() => { ... }`).
An arrow function that has an expression on the right hand side (`() =>
(...)`) will not be recognized.)
For example:
```ts
it("new_configurationclient", function () {
// @ts-ignore
const client = new ConfigurationClient(
process.env.ENDPOINT ?? "<app configuration endpoint>",
new DefaultAzureCredential()
);
});
```
The transpiler automatically "cleans" and validates the snippet using
similar techniques as the sample transpiler. As a result, it enforces
the same syntactic rules that the sample transpiler does. In addition to
those, it removes references to `process.env` (if an alternative is
specified), removes compiler pragmas like `// @ts-ignore`, and
automatically inserts imports for symbols that the snippet uses. So in
the above snippet, imports for `ConfigurationClient` and
`DefaultAzureCredential` are required, automatically detected, and
injected into the resulting snippet.
Snippets without `snippet:${name}` tags are _errors_ when using this
command, so a package must be fully migrated to use it.
### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?
### Are there test cases added in this PR? _(If not, why?)_
### Provide a list of related PRs _(if any)_
- #24536
### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_
### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
***NO_CI***
- replace version to "~5.6.2"
- rush update
- add `"skipLibCheck": true` for several packages that are affected by https://github.com/vitest-dev/vitest/issues/4688
- [formrecognizer] help TypeScript to understand with more explicit type
- [schema-registry-{avro,json}] skip lib check due to lru-cache issue with the Iterator TReturn change
https://github.com/isaacs/node-lru-cache/issues/348
There's some change in `typescript-eslint` that breaks some eslint test
cases.
Previously when running tests, in a rule's `create()` method, we get a
relative
path for `context.filename`, and when we pass a setting of `{ main:
"src/test.ts" }` in rule tester option, the two were considered the same
file,
therefore the following condition check failed, and we continue to
report error
for default exports.
6dd8b582e2/common/tools/eslint-plugin-azure-sdk/src/rules/ts-modules-only-named.ts (L31)
After the upgrade, `context.filename` now contains the full path, the
file
`src/test.ts` in our test fixture has a full path of
`/.../common/tools/eslint-plugin-azure-sdk/tests/fixture/src/test.ts`.
It is not
considered the same file as the passed settings implies
`/.../common/tools/eslint-plugin-azure-sdk/src/test.ts` when we run the
tests.
So we return earlier instead of going to check for default exports.
This is not an issue in real scenario where we have settings of `{ main:
"src/index.ts" }`, which would match the full path of the file we are
linting
when we run `eslint` from a package directory.
This PR fixes the test by passing `tests/fixture/src/test.ts` in the
settings so
that it matches the full path of `context.filename` we get in
`create()`.
- [eslint-plugin] github-source-headers rule should use "MIT License".
This change updates our linter rule and the whole repository to have consistent
license header.
***NO_CI***
- remove "All rights reserved." from header
- "MIT license" => "MIT License"
In our current setup, tshy won't build test and samples-dev folder even
they are
included in tsconfig.json. This PR adds a command that does the
type-checking
for all files included in tsconfig.json, and allow additional files to
be passed
in as well.
to be consistent with `eslint` version.
Also removed the eslint plugin helper. It was added to help
vscode-eslint
extension to resolve plugin packages. With flat config it is no longer
needed.
- adjust options passed `RuleTester` constructor
- remove `recommended` property that no longer exists
- use `output` of array to indicate multi-pass fixes with overlapping
range
- temporarily disable two test cases of ts-modules-only-named suite.
***NO_CI***
- bump typescript-eslint dependencies version to ~8.2.0
- [eslint-plugin] suppress new linting errors appearing with v8
- [eslint-plugin] react to removed typescript-eslint property
related PR https://github.com/typescript-eslint/typescript-eslint/pull/9025
- [eslint-plugin] turn off @typescript-eslint/no-unused-expressions
as no-unused-expressions is already reported by eslint
- [dev-tool] fix new linting errors
- Remove @typescript-eslint/ban-types that no longer exists
and replace with more specific rules wherever applicable.
- [cosmosdb] combine the test eslint config with the main one
- delete catch expression when caught error is not used
- fix or suppress linting errors in code files
Add a configuration to turn off all `eslint-plugin-n` rules for browser
code files.
Also remove the redundant `plugins` block in `nCustomization` because
the
plugin's "flat/recommended" config already does that
e5e758ea0c/lib/index.js (L87)
-------
### Packages impacted by this PR
eslint-plugin-azure-sdk
- add a config in eslint-plugin-azure-sdk for linting with type
information
- remove .eslint* and replace with eslint.config.mjs
- remove --ext from linting NPM scripts
- fix linting error: URL is now available in global scope on our
supported platforms.
***NO_CI***
- add one flat config file for perf tests
- update NPM scripts of these packages to use that flat config for perf tests
As eslint v8 still not switched to flat config by default, this change
temporarily uses `cross-env` to set `ESLINT_USE_FLAT_CONFIG=true`
Once we moved to eslint v9 the env var can be removed.
and replacing them with eslint.config.mjs so that both command line
eslint and
the vscode eslint extension can use the same config.
- since vite-plugin-browser-test-map doesn't depend on
eslint-plugin-azure-sdk, add linter dev dependencies and a flat config
to it.
- add an internal flat config to our eslint plugin. use it in added flat
config for test utility packages
Jeremy Meng
Maor Leger
Jeremy Meng
Matthew Podwysocki
Maor Leger
Minh-Anh Phan