fix: drop env reads and Object.is from dist to clear socket.dev alerts#3
Merged
Conversation
socket.dev flags any process.env read regardless of the var name. Remove env-based detection entirely — DEV is a compile-time true. Users call silenceWarnings() in their production entry point to opt out. The warning still fires at most once per process, so the cost is minimal. Closes socket.dev alert: Environment variables — dist/dev.js
socket.dev detects Object.is as a URL because .is is a valid ccTLD. Replace with "SameValue equality" in the union JSDoc (which compiles into dist/p.d.ts). Update READMEs to remove NODE_ENV mention and document silenceWarnings() as the only opt-out mechanism. Closes socket.dev alert: URL strings — Package overview
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Follow-up to #2 — both socket.dev alerts persisted on 1.0.1.
Root causes were different from what was assumed in the first pass.
Alert 1 — Environment variables (
dist/dev.js)socket.dev flags any
process.envread, includingNODE_ENV.Removed env-based dev detection entirely.
DEVis now a compile-timetrue— warnings are on by default and fire at most once per process.Users who want to suppress them call
silenceWarnings()in theirproduction entry point. READMEs updated accordingly.
Alert 2 — URL strings (
dist/p.d.ts, Package overview).isis a valid ccTLD (Iceland). socket.dev's URL scanner was treatingObject.isas a domain name in the compileddist/p.d.ts(TypeScriptpreserves JSDoc in declaration files). Replaced with "SameValue equality"
in the
P.unionJSDoc — same meaning, no false positive.Why
Both alerts survived the 1.0.1 publish. Score stayed at 77.
Checklist
silenceWarnings()behaviour unchanged.