Attempt to fix https://github.com/reactjs/react-router/issues/3241

[slorber]

No description provided.

[taion]

The 3rd arg here should just be omitted, e.g.

window.history.replaceState({ ...historyState, key }, null)

[slorber]

thanks yes I think it should work too, I'll try tomorow and edit my PR

[slorber]

@taion I've tested and yes it seems to work fine for my usecase with your suggestion so I've updated the PR

[slorber]

Just also want to point out that this code in useBasename.js:

    // Automatically use the value of <base href> in HTML
    // documents as basename if it's not explicitly given.
    if (basename == null && _ExecutionEnvironment.canUseDOM) {
      var base = document.getElementsByTagName('base')[0];

      if (base) basename = _PathUtils.extractPath(base.href);
    }

It does not work to automatically solve my usecase so I have to configure manually the basename. Anyway not sure there's anything clever to be done here. Maybe history could automatically detect that basename has a different domain than current url and issue a meaningful error to tell the user to configure a basename?

I tried if (base) basename = base.href; but it's not really what we want here because actually the history basename should be set to the backend server for solving the security issue, while my page has base=frontend server.

Something like that might work:

var base = document.getElementsByTagName('base')[0];
if (base ) {
  // For most history/RR users 
 // base can be a relative path, or an absolute path of the same domain as the current page
  if ( isLocationDomain(base) {
    basename = _PathUtils.extractPath(base.href);
  } 
  // For my usercase. Should not impact other users but make possible to solve my usecase without any configuration (with possibility to override if needed)
  else {
    basename = getLocationDomain();
  }
}

What do you think? I can make a new PR for that if you want.

[taion]

See the recent discussion on #94. The <base href> support in this library is just not correct and needs to be replaced entirely (e.g. with <base basename> or something).

[taion]

From #94 (comment) onward.

[slorber]

@taion @mjackson @ryanflorence I've explained my issue more in-depth in remix-run/react-router#3241 (comment)

Now I don't want to rush you guys but I have to put something in production in 8 days :) so I need to know if you will consider merging this fix and release 2.0.2 soon, or if I should consider publishing my own fork at least temporarily

Thanks

[taion]

@slorber Would you mind adding a regression test here? The triggers for the bug are obscure enough that is a nontrivial risk of a regression later on without such a test.

[slorber]

ok i'll try to check how this can be tested :) if you have ideas plz tell me

[taion]

Give me a sec.

[taion]

I added it: #274.

[slorber]

great thanks :)