Azure / autorest.typescript

Extension for AutoRest (https://github.com/Azure/autorest) that generates TypeScript code. The transpiled javascript code is isomorphic. It can be run in browser and in node.js environment.
MIT License
177 stars 75 forks source link

Use shared rollup config when generating rollup files #1259

Open maorleger opened 2 years ago

maorleger commented 2 years ago

As you can see here and in the usage of it here we have a shared rollup config that packages should use within the monorepo. This issue tracks updating the generated code to use the same shared config.

I believe https://github.com/Azure/autorest.typescript/blob/main/src/generators/static/rollupConfigFileGenerator.ts needs to be updated here, but I am not familiar with the TS generator to say what else needs updating

deyaaeldeen commented 2 years ago

@maorleger thanks for opening this. Currently, code gen generates a rollup config that mirrors what is in the shared one in the mono repo.

Once you merge your PR, feel free to open a PR here against the rollupConfigFileGenerator.ts module with your changes.

sarangan12 commented 2 years ago

@maorleger Am I correct in assuming that you want the rollup to be generated as "https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/formrecognizer/ai-form-recognizer/rollup.config.js"? Could you please confirm?

maorleger commented 2 years ago

@maorleger Am I correct in assuming that you want the rollup to be generated as "https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/formrecognizer/ai-form-recognizer/rollup.config.js"? Could you please confirm?

Yeah, that's what I was thinking originally - would that be a reasonable thing to do here?

deyaaeldeen commented 2 years ago

@maorleger, currently code gen does not add dev-tool to the devDependencies list because the code gen smoke tests build generated packages and dev-tool is not published. The current approach is to just duplicate the config file until code gen tests gain the ability to checkout the mono repo and install unpublished deps from there. See https://github.com/Azure/autorest.typescript/pull/954.

maorleger commented 2 years ago

@maorleger, currently code gen does not add dev-tool to the devDependencies list because the code gen smoke tests build generated packages and dev-tool is not published. The current approach is to just duplicate the config file until code gen tests gain the ability to checkout the mono repo and install unpublished deps from there. See #954.

Oh thank you for the context here! Was not aware... do you think it makes sense to keep this issue open but on the backburner or just close it? I'm fine either way, as long as the autogen crew had a chance to opine on it 😄

sarangan12 commented 2 years ago

@maorleger We could do it in other ways too. We could have a flag such as -mono-repo which is set to false by default. (This flag could be used for things such as rollup, tsconfig, rush, etc) So, there will not be any impact to test packages and acual production packages also could be generated easily.

dw511214992 commented 2 years ago

@sarangan12 I think --mono-repo should be set to true by default, because we don't hope to add --mono-repo in generating codes for azure-sdk-for-js.

witemple-msft commented 2 years ago

@sarangan12 I'm going to pick this up in March or so (maybe sooner if it's very simple). Do you feel we need to have an isolated (non-monorepo) option for Track 2 generated Azure SDK packages? I could pretty simply update the static generator to depend on dev-tool and use its commands but want to make sure that won't break folks who aren't participating in the rush workspace.

sarangan12 commented 2 years ago

By default we could generate for mono repo. We could have an option introduced to follow the non mono repo

dw511214992 commented 2 years ago

@sarangan12 @witemple-msft I have added a flag --mono-repo in https://github.com/Azure/autorest.typescript/pull/1309 . The flag is set to be true by default, and then we don't need change the command to generate codes for azure-sdk-for-js. In codegen test, we need to add --mono-repo=false in the command.

The PR only contains the changes for RLC, I think you can change the parts of generating regular js sdk. Thanks

dw511214992 commented 2 years ago

@sarangan12 In @joheredi 's comment, flag azure-sdk-for-js may be preferred. What do you think about it? thanks

sarangan12 commented 2 years ago

Note: We are continuing the conversation in the PR comment.

dw511214992 commented 2 years ago

@sarangan12 @joheredi @qiaozha find there is another option is-test-package, and not sure whether it meets this issue's requirement. thanks