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

Feature for excluding names #29

Closed
wants to merge 2 commits into from
Closed

Feature for excluding names #29

wants to merge 2 commits into from

Conversation

CalmarBen
Copy link

publicJSDocTag does not meet all use case I have.
For example I use some code like this:
this.Add('rect', { x:0, y:0, width: rectWidth, height: rectHeight, class: 'rec_class'} );

I don't want to quote every item like this (this is less readable)
this.Add('rect', { 'x':0, 'y':0, 'width': rectWidth, 'height': rectHeight, 'class': 'rec_class'} );

In this usecase I think it is more flexible to use an exlusion from a list, like this in webpack.config.js:

          getCustomTransformers: program => ({
              before: [
                  propertiesRenameTransformer(program, { entrySourceFiles: [ './app.ts', './webapp/webapp.ts'],
                  exclusionPattern: [ 'class', 'x', 'y', 'width', 'height'],
                  })
              ]
          })

@CalmarBen
Copy link
Author

I have also updated package dependencies as it prompted vulnerabilities

@timocov
Copy link
Owner

timocov commented Mar 7, 2022

I don't want to quote every item like this (this is less readable)

@CalmarBen You don't need to, I don't even think that it will prevent these properties from renaming though (if it does then I'd fix it 😄).

To prevent minifying these properties a type of that object should be publicly accessible or marked as "public" via jsdoc. Do you have c complete example of this issue in your project or is it possible to create a repro? I believe it should be fixed in different even without modifying the code of the tool.

This option might be useful for loose mode (see #14), we don't have it yet.

@CalmarBen
Copy link
Author

CalmarBen commented Mar 7, 2022

Thanks for looking at my PR +1:
In this usecase I use no class or interface declaration. This is when putting attributes to the DOM, using the following method:

protected static createHtmlElem<K extends keyof HTMLElementTagNameMap>(qualifiedName: K, attr?: { [k: string]: any }, text?:string): HTMLElementTagNameMap[K] {
        const result: HTMLElementTagNameMap[K] = document.createElement(qualifiedName)
        if (attr instanceof Object) {
            for (const attrName in attr) {
                result.setAttribute(attrName, attr[attrName])
            }
        }
        if (text !== undefined) {
            result.textContent = text
        }
        return result
    }

used like this, for example:
this.createHtmlElem('p', { class: 'debug' });
Of course I don't want 'class' item to be minimized.
I notice that if I quote the attribute it is not (and I like it!):
this.createHtmlElem('p', { 'class': 'debug' });

@timocov
Copy link
Owner

timocov commented Mar 7, 2022

Have you tried to replace attr?: { [k: string]: any } with attr?: Record<string, any>? I believe this might work because Record is an interface declared in public space.

@timocov
Copy link
Owner

timocov commented Mar 7, 2022

I notice that if I quote the attribute it is not (and I like it!):

I'd say this is a bug tbh 😂 I'll fix this later.

I think that this is a bug because this tool relies on the type information rather than the syntax so whatever syntax you use it should work the same IMO.

@CalmarBen
Copy link
Author

CalmarBen commented Mar 7, 2022

Have you tried to replace attr?: { [k: string]: any } with attr?: Record<string, any>? I believe this might work because Record is an interface declared in public space.

You are right, this is working. This is nice tip, I think I would never find out to solve it this way. Thanks a lot @timocov

@CalmarBen
Copy link
Author

I think I can remove my PR ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants