Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing default configuration options #9945

Closed
glowcloud opened this issue May 17, 2024 · 6 comments
Closed

Add missing default configuration options #9945

glowcloud opened this issue May 17, 2024 · 6 comments

Comments

@glowcloud
Copy link
Contributor

There are configuration options, such as modelPropertyMacro and parameterMacro, which do not have any default values defined in:

const defaultOptions = Object.freeze({

We need to identify all of the missing options and assign them default values.

@glowcloud
Copy link
Contributor Author

The missing configs are:

  • operationsSorter
  • tagsSorter
  • onComplete
  • modelPropertyMacro
  • parameterMacro

@char0n
Copy link
Member

char0n commented May 18, 2024

I think there is additional request.curlOptions as well. But needs to be researched first.

@glowcloud
Copy link
Contributor Author

glowcloud commented May 20, 2024

request.curlOptions is set inside of requestInterceptor:

This can be set on the mutated request in the requestInterceptor function. For example request.curlOptions = ["-g", "--limit-rate 20k"]

The default for it is:

 requestInterceptor: (a) => a

so perhaps it should be:

requestInterceptor: (request) => {
   request.curlOptions = null;
   return request;
}

@glowcloud
Copy link
Contributor Author

glowcloud commented May 20, 2024

Potential default values for each of them:

  • onComplete: () => {} - defined as Function=NOOP in documentation, used here:
    updateJsonSpec: (ori, system) => (...args) => {
    const cb = system.getConfigs().onComplete
    if(engaged && typeof cb === "function") {
    // call `onComplete` on next tick, which allows React to
    // reconcile the DOM before we notify the user
    setTimeout(cb, 0)
    engaged = false
    }
    return ori(...args)
    }
  • modelPropertyMacro: null - so that we don't run visitors / plugins for these
  • parameterMacro: null
  • requestInterceptor / request.curlOptions:
    (request) => {
       request.curlOptions = [];
       return request;
    }
  • operationsSorter: null
  • tagsSorter: null

operationsSorter and tagsSorter are used here:

export const taggedOperations = (state) => ({ getConfigs }) => {
let { tagsSorter, operationsSorter } = getConfigs()
return operationsWithTags(state)
.sortBy(
(val, key) => key, // get the name of the tag to be passed to the sorter
(tagA, tagB) => {
let sortFn = (typeof tagsSorter === "function" ? tagsSorter : sorters.tagsSorter[ tagsSorter ])
return (!sortFn ? null : sortFn(tagA, tagB))
}
)
.map((ops, tag) => {
let sortFn = (typeof operationsSorter === "function" ? operationsSorter : sorters.operationsSorter[ operationsSorter ])
let operations = (!sortFn ? ops : ops.sort(sortFn))
return Map({ tagDetails: tagDetails(state, tag), operations: operations })
})
}

Considering that they are supposed to be sorting functions, I think we should have them as null so that they don't run at all. In our configuration, it says that they are: Function=(a => a) and making that default wouldn't sort anything as well, so it's also an option.

@char0n
Copy link
Member

char0n commented May 21, 2024

Yes, good choice for the defaults. I would also go for a nullable-function (typecaster) for the onComplete for consistency. There is a check in code present to assert if it's a function.

For the curlOptions, as established in the PR already, we want to go for:

requestInterceptor: (request) => {
   request.curlOptions = [];
   return request;
}

char0n added a commit that referenced this issue May 21, 2024
Refs #9945

---

Co-authored-by: Vladimír Gorej <vladimir.gorej@gmail.com>
@char0n
Copy link
Member

char0n commented May 21, 2024

Addressed in #9949

@char0n char0n closed this as completed May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants