[DevTool] sample publish doesn't work

[qiaozha]

it will report errors like this

when you run dev-tool samples publish -f now.

which is probably caused by https://github.com/Azure/azure-sdk-for-js/pull/19496/files this PR.

[jeremymeng]

@qiaozha the error means that ?. syntax is not supported in the lowest version of platforms that we support. it should be replaced with something like

if (dataSources.body.entityDefs) {
  console.log(dataSources.body.entityDefs.map(...)
}

@witemple-msft there seems to be some minor bug in the suggested replacement.

[witemple-msft]

@jeremymeng Yes, the suggestions can only go so far if the expression is highly compound. Your alternative is probably what I'd write as well.

The other two errors are caused by this import x from "module" syntax. In ECMAScript module syntax, this is a default import, but because of esModuleInterop we can't know whether it is actually importing the default symbol or if it's equivalent to import * as x from "module";, so default imports are not allowed for external modules anymore.

This can be fixed by rewriting them as import * as dotenv from "dotenv" and import * as PurviewCatalog from "@azure-rest/purview-catalog";

@qiaozha I will take a look through these samples and make a PR.

[qiaozha]

as this default export is related to RLC. @joheredi can you take a look at it ? Thanks

[joheredi]

@witemple-msft we use default exports for Rest Clients. How should we import in our samples?

import * as PurviewCatalog from "@azure-rest/purview-catalog";
const PurviewCatalogClient = PurviewCatalog.default;

Would it be possible to add some metadata to the default import to tell the dev-tool we are actually doing a default import vs a * as import?

[witemple-msft]

@joheredi Do you think it's safe to simply assume that if the package's namespace is @azure-rest that we can allow it? Basically it would just be extending the allowed default imports.

The problem is that without the metadata you're describing, it's impossible for me to know if a "Default Import" in typescript represents a true ES default import (binding the export named default), or an interop default import that evaluates to module.exports.

[witemple-msft]

Currently I only allow it for relative modules and node builtins, but we could add an allow list or something if it's really necessary, but I think the best solution is to just decide which Azure SDK packages to allow to use default exports. They're forbidden in convenience-layer SDKs, for example (because of how they interact strangely with CommonJS).

[qiaozha]

@joheredi Should we remove the default export in RLC if that's the case ?

[joheredi]

@witemple-msft I think it would be safe to allow default imports for @azure-rest. I'd be okay also if we disallow by default and add metadata in our samples something like. Either way should be okay

// @dev-tool default-import
import FooClient from "@azure-rest/foo

@qiaozha I don't think we should remove default exports for RLC, it looks like the dev-tool could whitelist @azure-rest or take metadata to allow default imports in the samples

[witemple-msft]

@joheredi Alright. I'll put up a patch to allow default imports for anything in any @azure-rest/* package since it's part of the programming pattern in RLC. I'm hesitant to add support for the metadata item because it will introduce a lot of extra comment-parsing that we don't currently have.