replaceAll

[char0n]

https://github.com/tc39/proposal-string-replaceall

Currently in stage 2. Should be fairly safe to implement.

[char0n]

This has reached stage 3. Possibly to replace our custom implementation with future polyfill. One things was added there and that is handling of empty strings: https://github.com/tc39/proposal-string-replaceall#what-happens-if-searchvalue-is-the-empty-string

We need to make sure our implementation compensate for that.

[js636f]

@char0n, so, we need this:

'xxx'.replaceAll('', '_');
// → '_x_x_x_'

now we have this:

RA.replaceAll('', '_', 'xxx');
// → 'x_x_x'

I think we may change this:

const replaceAll = curryN(3, (searchValue, replaceValue, str) => {
  if (isRegExp(searchValue)) {
    throw new Error('searchValue must be a String, not a RegExp.');
  }

  return pipe(
    split(String(searchValue)),
    join(String(replaceValue))
  )(str);
});

to something like this:

const replaceAll = curryN(3, (searchValue, replaceValue, str) => {
  if (isRegExp(searchValue)) {
    throw new Error('searchValue must be a String, not a RegExp.');
  }

  const callIfsearchValueIsEmpty = () => {
    return String(str).replace(/(?:)/g, String(replaceValue));
  }

  if (searchValue==='') 
    return callIfsearchValueIsEmpty();

  return pipe(
    split(String(searchValue)),
    join(String(replaceValue))
  )(str);

});

and add this to the test cases:

it('should correctly handle empty searchValue case', function() {
    const value = 'xxx';
    const actual = RA.replaceAll('', '_', value);
    const expected = '_x_x_x_';

    assert.strictEqual(actual, expected);
  });

If these ideas look reasonable - I'm ready to prepare and make a PR.

[char0n]

@js636f your assumptions are correct. But we need a little bit different strategy. If we get this right we might be the first reference implementation. So let's do it in the following way:

1.) Ponyfill

Create a ponyfil in our ponyfill directory. This ponyfill must not use anything from ramda or ramda-adjunct bust mu be pure javascript, so that it's framework agnostic.

2.) Implementation

Then as usually, we check for String.prototype.replaceAll and use it if present, otherwise use ponyfill.

Here is an example how ponyfill can look like: https://twitter.com/_developit/status/1097302964274892800?lang=en (out of date - stage 1)

I'm assigning this issue to you, own it, if you wish!

[char0n]

@js636f would be good for you to watch this issue: tc39/proposal-string-replaceall#29

[js636f]

@char0n, OK, thank you! I'm working on this.

[js636f]

This is my implementation of the base code.

let replaceAllPonyFill = (str, searchValue, replaceValue) => {

  if (str === undefined || 
      str === null ||
      searchValue === undefined ||
      searchValue === null ||
      replaceValue === undefined ||
      replaceValue === null) {
        throw TypeError('Input values must not be `null` or `undefined`');
    }

    if (typeof str !== 'string' ||
        typeof replaceValue !== 'string') {
          throw TypeError('`str` and `replaceValue` must be strings');
    }

    if (typeof searchValue !== 'string' && 
        !(searchValue instanceof RegExp)) {
          throw TypeError('`searchValue`  must be a string or an regexp');
    }

  //searchValue is an empty string
  if (searchValue==='') return str.replace(/(?:)/g, replaceValue);
  
  //searchValue is a global regexp
  if (searchValue instanceof RegExp){
    if (searchValue.flags.indexOf('g')===-1){
      throw TypeError('`.replaceAll` does not allow non-global regexes');
    }
    return str.replace(searchValue, replaceValue);
  }

  //common case
  return str.split(searchValue).join(replaceValue);
}

let text = 'xxx';

console.log(replaceAllPonyFill(text,'x','v'));
//vvv

console.log(replaceAllPonyFill(text,'','_'));
//_x_x_x_

console.log(replaceAllPonyFill(text,/x/g,'v'));
//vvv

//console.log(replaceAllPonyFill(text,/x/,'v'));
//TypeError

//console.log(replaceAllPonyFill(555,'5','v'));
//TypeError

//console.log(replaceAllPonyFill(text,5,'v'));
//TypeError

//console.log(replaceAllPonyFill(text,'x',5));
//TypeError

//console.log(replaceAllPonyFill(null,'x','v'));
//TypeError

It seems to me it's working and it's in line with the proposal.

If it looks OK, then I'm ready to make a PR, to refine it and to change existing replaceAll.js-related things. And I think we need to make a ponyfill folder (I can't find it) and perhaps we should write dedicated tests for such ponyfills (like in the new folder test/ponyfills).

[char0n]

Implementation seems about right. Did you base your work on https://github.com/zloirock/core-js/blob/v3.3.2/packages/core-js/modules/esnext.string.replace-all.js ?

Regarding ponyfill vs polyfill. When this library was born, we refereed to all the ponyfills incorectly as polyfills. We have a standing issue which should solve this later. So for now (for consistency), put you ponyfill into src/internal/polyfills directory. Usually when we create a ponyfill we test it directly in the function that's using it as demonstrated here. Because the reasoning it that in any other code we want to use our on RA.replaceAll and not the naked ponyfill.

[js636f]

My implementation is mainly based on the proposal and on this implementation (global regex check, TypeError exception), I think I understand the structure, so, I'm working on the PR.

[js636f]

I made the PR.

[char0n]

Another polyfill https://npmjs.com/string.prototype.replaceall