Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

feat: add ability to perform package override in config #2077

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

wtrocki
Copy link
Contributor

@wtrocki wtrocki commented Sep 18, 2020

@wtrocki
Copy link
Contributor Author

wtrocki commented Sep 18, 2020

#2076

@craicoverflow craicoverflow self-requested a review September 18, 2020 11:31
Copy link

@craicoverflow craicoverflow left a comment

Choose a reason for hiding this comment

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

src/loadPlugins.ts(16,8): error TS2304: Cannot find name 'config'.
src/loadPlugins.ts(17,20): error TS2304: Cannot find name 'config'.

Did you run this? Want me to take it over?

@craicoverflow
Copy link

Pushed some changes to fix this, will add documentation now also.

@wtrocki
Copy link
Contributor Author

wtrocki commented Sep 18, 2020

I'm looking if that is something you would accept. I see couple issues:

  • It looks like we completely removed the graphql config side of things from the docs
  • Currently config is used only for client side generation.
  • Would this approach work for you?

I have made change in node_modules and ported it to repo (but improperly).

@craicoverflow
Copy link

Would this approach work for you?

Yes, although I changed the property name from packageOverride to packageNameOverride to be more declarative.

I will look to add docs change in another PR and we can get this merged and released.

@wtrocki
Copy link
Contributor Author

wtrocki commented Sep 18, 2020

@craicoverflow - I found way out. If I define output file in runtime (schema plugin) I will get schema updated so this is not priority but when we land this I would be able to do proper setup in datasync starter.

Thank you so much!

Copy link
Contributor Author

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

Cannot approve but amazing work

let pluginName = pluginLabel;
if (pluginLabel.startsWith('graphback-')) {
// Internal graphback plugins needs rename
pluginName = pluginLabel.replace('graphback-', '@graphback/codegen-');
}
// override package name
if(pluginConfigMap[pluginLabel].packageNameOverride){
pluginName = pluginConfigMap[pluginLabel].packageNameOverride;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
pluginName = pluginConfigMap[pluginLabel].packageNameOverride;
pluginName = pluginConfigMap[pluginLabel].packageName

Choose a reason for hiding this comment

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

Updated now in 0c2dcd6

@craicoverflow
Copy link

Cannot approve but amazing work

I can approve but should not approve myself, @machi1990 mind taking a look?

@craicoverflow craicoverflow changed the title fix: add ability to perform package override in config feat: add ability to perform package override in config Sep 21, 2020
@craicoverflow craicoverflow merged commit 2cb0902 into master Sep 21, 2020
@craicoverflow craicoverflow deleted the package-override branch September 21, 2020 08:32
@craicoverflow craicoverflow linked an issue Sep 21, 2020 that may be closed by this pull request
@craicoverflow craicoverflow added the enhancement New feature or request label Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datasync cannot be used as plugin for generating schema to file.
3 participants