Skip to content

[...].join() - handle null and better compress [...].join("str")#301

Closed
Skalman wants to merge 1 commit intomishoo:masterfrom
Skalman:join-null
Closed

[...].join() - handle null and better compress [...].join("str")#301
Skalman wants to merge 1 commit intomishoo:masterfrom
Skalman:join-null

Conversation

@Skalman
Copy link
Copy Markdown
Contributor

@Skalman Skalman commented Sep 23, 2013

  • null, undefined, and holes in arrays are interpreted as an empty
    string by Array.prototype.join. This commit fixes various issues when
    compressing them, or values that can be null or undefined.
  • Compress [foo+"str","123"+bar].join("-") -> foo+"str-123"+bar

The only issue that I can see now is that [ "foo", bar + "baz" ].join("-") produces "foo-"+(bar+"baz"). I might fix that too.

Btw, thanks for moving these out of evaluate(). I find it difficult to know where to add code sometimes.

* null, undefined, and holes in arrays are interpreted as an empty
  string by Array.prototype.join. This commit fixes various issues when
  compressing them, or values that can be null or undefined.
* Compress [foo+"str","123"+bar].join("-") -> foo+"str-123"+bar
@pjeweb
Copy link
Copy Markdown

pjeweb commented Nov 19, 2015

+1 Holes is a problem for me too when compressing js-cookie (js-cookie/js-cookie#110).

@kzc
Copy link
Copy Markdown
Contributor

kzc commented Nov 19, 2015

Original [1, undefined, 2].join('') problem seen in js-cookie/js-cookie#110 stems from the use of the unsafe=true in the uglify compress options. This option is disabled by default. Some upstream uglify wrapper library must be enabling it.

$ uglifyjs -V
uglify-js 2.6.1

$ echo "console.log([1, undefined, 2].join(''));" | uglifyjs -c "unsafe=true" | node
1undefined2

$ echo "console.log([1, undefined, 2].join(''));" | uglifyjs -c "unsafe=false" | node
12

$ echo "console.log([1, undefined, 2].join(''));" | uglifyjs -c | node
12

$ echo "console.log([1, undefined, 2].join(''));" | node
12

@pjeweb
Copy link
Copy Markdown

pjeweb commented Nov 20, 2015

oops. you're right. completely missed that

@rvanvelzen
Copy link
Copy Markdown
Collaborator

This is probably outdated, so I'm closing this PR. Feel free to re-submit if you think it should be merged.

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.

4 participants