Skip to content

[Feature] Move duplicate prop check to a test instead of runtime #75

@colbyfayock

Description

@colbyfayock

Feature Request

From Josh Goldberg (thanks)

There is logic right now to assert that no two plugins declare the same prop:

const propsCheck: string[] = [];

transformationPlugins.forEach(({ props = [] }) => {
  props.forEach((prop) => {
    if (propsCheck.includes(prop)) {
      throw new Error(`Option ${prop} already exists!`);
    }
    propsCheck.push(prop);
  });
});

This seems reasonable. I’d suggest doing this at test time so that runtime users don’t need to run that code.

I’d also personally lean towards a Set for an ever so slightly faster check (not urgent at all):

const propsCheck = new Set<string>()

transformationPlugins.forEach(({ props = [] }) => {
  props.forEach(prop => {
    if ( propsCheck.has(prop) ) {
      throw new Error(`Option ${prop} already exists!`);
    }
    propsCheck.add(prop);
  });
});

Solution

Create a new test that checks all of the plugins for duplicate props. It can use the logic above to perform the check.

Then remove this check from the constructCloudinaryUrl function

Metadata

Metadata

Assignees

No one assigned

    Labels

    help wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions