Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upmake component names case insensitive. ie: getComponent("fooBaR") #3393
Comments
I think this is a very good idea |
Hi there! I’m a first-time contributor and was hoping to help out with this issue. I noticed nobody was assigned to it, but if there’s already a solution in progress I’m happy to un-assign myself and try helping out elsewhere. Thanks! |
@devthehuman, thanks for checking in! Nobody's working on this, afaik. Feel free to open a pull request that solves this! Comment here if you need any clarification. |
Thanks! Just a quick question about this first bullet point: "When building the system, we store the downcased name of the component in our object." Could you point me in the right direction for where in the build process this should be happening? Is the object you're referring to this base file? |
hey @devthehuman ... the file ( and function, I think ) you're after lies here... https://github.com/swagger-api/swagger-ui/blob/master/src/core/system.js#L285 That function wraps deepExtend, which is how we combine plugins into one largish object. if(isObject(src.components)) { // Bringing in new components?
dest.components = {...dest.components, downcaseThePropsOf(src.components) } // downcaseTheProps doesn't exist btw.
delete src.components // so that we don't add it _again_ later down in this function
} There is likely a prettier way, but that's the safest area to add the code, as it's run when we add stuff to the system ( ie: the big object ). Hope that helps. Stick some console.logs in there to see the stuff that goes through it :) |
@devthehuman, not quite - that file holds the data going into the system "System" is our name for a structure we use to compile all our plugins and presets into one object that will be used to drive the React app. One part of the object is a Also, for the getting side of things - you'll want to look at the |
I'd like to increase the scope a bit here, due to the issue encountered in #3968.. We should be down casing any usage of component names in |
@shockey I believe there should be a better way; not everyone prefer case-insensitiveness. What about introducing a case check in dev environment and verbose the case miss-match on build. These additional code could be excluded from the production build by setting NODE_ENV and adding if blocks, which will be removed by uglifyjs as dead code; similar way as react remove propsType check. |
The plugin system exposes a way to get components via
system.getComponent()
.Up till now its been case sensitive. Unfortunately we never put in a style-guide on what case to use.
So we have lowercase, TitleCase components mixed throughout.
I propose we make them case-insensitive.
eg:
This is to allow any case, and not break any existing code. That said, we should likely normalize the component names in this codebase to be consistent.
Guestimate of work effort: < Normal
@shockey your thoughts?