Rename transform toggle ignore list

[cweckesser]

🚨 IMPORTANT: Please do not create a Pull Request without creating an issue first.

Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of
the pull request.

Description

The rename transform uses an ignore list by default to skip renaming certain types (e.g.: scalars). This could cause potential collisions if more than one schema defines the same scalars, since those cannot be renamed to avoid the mentioned collisions.

A flag was added to the rename transform configuration, which is true by default so as to keep backwards compatibility, which allows to toggle the usage of the ignore list.

Fixes #5341

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

Screenshots/Sandbox (if appropriate/relevant):

Adding links to sandbox or providing screenshots can help us understand more about this PR and take
action on it as appropriate

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration

• Unit test that checks that renaming of scalars can be achieved when the flag for using the ignore list is set to false

Test Environment:

OS: macOS Ventura 13.3.1 (22E261)
@graphql-mesh/transform-rename: 0.14.23
NodeJS: v16.18.0

Checklist:

• I have followed the
  CONTRIBUTING doc and the
  style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests and linter rules pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose
the solution you did and what alternatives you considered, etc...

[changeset-bot]

🦋 Changeset detected

Latest commit: 23b3926

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 109 packages

Name	Type
@graphql-mesh/transform-rename	Major
@graphql-mesh/types	Major
typescript-location-weather-example	Patch
type-merging-batching-example	Patch
@graphql-mesh/cli	Patch
@graphql-mesh/config	Major
@graphql-mesh/http	Major
@graphql-mesh/jit-executor	Major
json-machete	Major
@graphql-mesh/runtime	Major
@graphql-mesh/store	Major
@graphql-mesh/utils	Major
@graphql-mesh/cache-cfw-kv	Major
@graphql-mesh/cache-file	Major
@graphql-mesh/cache-localforage	Major
@graphql-mesh/cache-redis	Major
@graphql-mesh/graphql	Major
@graphql-mesh/grpc	Major
@graphql-mesh/json-schema	Major
@graphql-mesh/mongoose	Major
@graphql-mesh/mysql	Major
@graphql-mesh/neo4j	Major
@graphql-mesh/odata	Major
@graphql-mesh/openapi	Major
@graphql-mesh/postgraphile	Major
@graphql-mesh/raml	Major
@graphql-mesh/soap	Major
@graphql-mesh/thrift	Major
@graphql-mesh/tuql	Major
@graphql-mesh/transform-cache	Major
@graphql-mesh/transform-encapsulate	Major
@graphql-mesh/transform-extend	Major
@graphql-mesh/transform-federation	Major
@graphql-mesh/transform-filter-schema	Major
@graphql-mesh/transform-hive	Major
@graphql-mesh/transform-hoist-field	Major
@graphql-mesh/transform-naming-convention	Major
@graphql-mesh/transform-prefix	Major
@graphql-mesh/transform-prune	Major
@graphql-mesh/transform-rate-limit	Major
@graphql-mesh/transform-replace-field	Major
@graphql-mesh/transform-resolvers-composition	Major
@graphql-mesh/transform-transfer-schema	Major
@graphql-mesh/transform-type-merging	Major
@graphql-mesh/merger-bare	Major
@graphql-mesh/merger-federation	Major
@graphql-mesh/merger-stitching	Major
@graphql-mesh/plugin-deduplicate-request	Major
@graphql-mesh/plugin-hive	Major
@graphql-mesh/plugin-http-cache	Major
@graphql-mesh/plugin-http-details-extensions	Major
@graphql-mesh/plugin-http2	Major
@graphql-mesh/plugin-live-query	Major
@graphql-mesh/plugin-mock	Major
@graphql-mesh/plugin-newrelic	Major
@graphql-mesh/plugin-operation-field-permissions	Major
@graphql-mesh/plugin-prometheus	Major
@graphql-mesh/plugin-rate-limit	Major
@graphql-mesh/plugin-response-cache	Major
@graphql-mesh/plugin-snapshot	Major
@graphql-mesh/plugin-statsd	Major
@omnigraph/json-schema	Major
@omnigraph/openapi	Major
@omnigraph/soap	Major
auth0-example	Patch
cloudflare-workers	Patch
example-gcp	Patch
graphql-file-upload-example	Patch
grpc-example	Patch
grpc-reflection-example	Patch
hasura-openbrewery-geodb	Patch
hello-world-esm	Patch
json-schema-hello-world	Patch
covid-mesh	Patch
json-schema-example	Patch
json-schema-fhir	Patch
json-schema-file-upload	Patch
json-schema-subscriptions	Patch
mongoose-example	Patch
mysql-employees	Patch
mysql-rfam	Patch
neo4j-example	Patch
nextjs-apollo-example	Patch
nextjs-sdk-example	Patch
odata-microsoft-graph-example	Patch
odata-msgraph-programmatic-ts	Patch
odata-msgraph-programmatic	Patch
odata-trippin-example	Patch
javascript-wiki	Patch
openapi-meilisearch	Patch
openapi-stackexchange	Patch
openapi-stripe	Patch
openapi-subscriptions	Patch
openapi-youtrack	Patch
openwhisk-example	Patch
postgres-geodb-example	Patch
programmatic-batching-example	Patch
reddit-example	Patch
country-info-example	Patch
soap-demo	Patch
soap-netsuite	Patch
spacex-cfw	Patch
chinook	Patch
thrift-calculator	Patch
federation-gateway	Patch
gateway-example	Patch
@omnigraph/raml	Major
@graphql-mesh/apollo-link	Major
@graphql-mesh/urql-exchange	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ardatan]

I understand the use case but not sure if that flag should be in that way. Maybe includeDefaults???

[cweckesser]

I understand the use case but not sure if that flag should be in that way. Maybe includeDefaults???

@ardatan Do you mean the flag should be renamed to includeDefaults? If that's the case, then it's default value should be false, meaning that the ignore list will be enforced when renaming types (and thus, keeping backwards compatibility).

[ardatan]

Other than that, I don't see any point of toggling ignore list. If defaults are the case, the flag should be for defaults. If not, it doesn't make sense to enable and disable ignore list.

[cweckesser]

@ardatan I just pushed the changes I mentioned in my last comment (renaming the useIgnoreList flag to includeDefaults). I also wanted to say that don't fully understand you last comment:

If defaults are the case, the flag should be for defaults. If not, it doesn't make sense to enable and disable ignore list.

What do you mean with this?

[ardatan]

I mean if the default values in the ignore list such as native scalars and so on, the new flag should cover them not the entire ignore list. The ignore list is something provided by the user, adding another flag that toggles its behavior is extra.
So includeDefaults or whatever flag should remove predefined values from the ignore list or not.

To summarize, if the flag disables the current behavior, UUID etc should be renamed but the type names given in ignore by the user should not.

[cweckesser]

TL;DR: This PR does not provide for modifying the default ignore list, but only to disable its application. Does this make sense to you?

The ignore list is already defined in the rename transform implementation: packages/transforms/rename/src/shared.ts. The PR aims just to toggle whether it is being applied or not. I agree that it might be useful in some cases to provide a custom ignore list. For us to proceed adopting mesh, we only require skipping the ignore list (or, as an alternative, the possibility to plug in custom transforms).

[gilgardosh]

@ardatan This case is valid. as the override is for the hard-coded list of scalars, not user-defined