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

Replaced the redundant map function and removed the unused variable #1919

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

DrKiraDmitry
Copy link
Contributor

@DrKiraDmitry DrKiraDmitry commented Jan 31, 2024

  1. Hi, i was just looking for how to improve your library, and see strange string when we replace all items on null, i try find when it's need and why we save our array but with null items

Motivation, if we replacy all items, on null, we try to save sourcesContent array length but for what

I only found one place when we use length, when check Content

postcss/lib/previous-map.js

Lines 133 to 138 in 219dd75

withContent() {
return !!(
this.consumer().sourcesContent &&
this.consumer().sourcesContent.length > 0
)
}

But you don't have this case in unit test, and i don't know need thik about this case because all case when we use this func we after nofing do with this data

All rigth, let's imagine what we really check length, next question "we only check wath our array is not empty" or "we use length of array for calcuted/itteration"?
It's important, because it's potential for optimization
If we use after that itterator based on sourcesContent length, we'll leave map.sourcesContent.map(() => null), but if we only check wath our array is not empty, we can create constant operation and write just [ null ]

Also i check code in "source-map-js" and applySourceMap() don't work with that key

P.s. i'm not understand why in a test:size we get the result
map.sourcesContent.map(() => null) => 14.66kB
null => 14.67kB

  1. this.customProperty = false, i just see what it's not using on your documentation and project code

@ai
Copy link
Member

ai commented Feb 1, 2024

Thanks

@ai ai merged commit 1a906e5 into postcss:main Feb 1, 2024
8 checks passed
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