Skip to content

Commit

Permalink
[docs] Fix render in dev mod in IE 11
Browse files Browse the repository at this point in the history
  • Loading branch information
@oliviertassinari
oliviertassinari committed Dec 10, 2019
1 parent 1492b7e commit 0de54dd
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion docs/src/modules/redux/initRedux.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ function create(initialState) {
process.browser &&
!window.__REDUX_DEVTOOLS_EXTENSION__ &&
// redux-logger needs this feature
Object['assign'] // eslint-disable-line dot-notation
// eslint-disable-next-line no-eval
eval('Object.assign')

This comment has been minimized.

Sign in to view

Copy link
@eps1lon

eps1lon Dec 11, 2019

Member

Object.hasOwnProperty('assign') === true

This comment has been minimized.

Sign in to view

Copy link
@oliviertassinari

oliviertassinari Dec 11, 2019

Author Member

Thanks, I will try it out

This comment has been minimized.

Sign in to view

Copy link
@eps1lon

eps1lon Dec 11, 2019

Member

I would sleep better at night knowing we don't have any eval 😉

Otherwise destructuring first and then type checking that variable should work too according to the docs: https://babeljs.io/docs/en/babel-plugin-transform-object-assign#caveats

This comment has been minimized.

Sign in to view

Copy link
@oliviertassinari

oliviertassinari Dec 11, 2019 •
edited
Loading

Author Member

Object['assign'] is replaced with _babel_runtime_corejs2_core_js_object_assign__WEBPACK_IMPORTED_MODULE_1___default.a. I haven't looked further.

Object.hasOwnProperty('assign') works, it's definitely cleaner. Committing.

This comment has been minimized.

Sign in to view

Copy link
@eps1lon

eps1lon Dec 11, 2019

Member

Ah yes that's transform-runtime not transform-object-assign.

This comment has been minimized.

Sign in to view

Copy link
@zloirock

zloirock Dec 11, 2019

Object.hasOwnProperty('assign') === true

Good example. Need to add transpiling this case too to Babel.

This comment has been minimized.

Sign in to view

Copy link
@eps1lon

eps1lon Dec 12, 2019

Member

Wait why would you transpile this? The goal is to not transpile this. We don't want to enter whatever is in that block in environments where Object.assign is not available.

This comment has been minimized.

Sign in to view

Copy link
@zloirock

zloirock Dec 12, 2019 •
edited
Loading

Because it should work with like with native Object.assign.

This comment has been minimized.

Sign in to view

Copy link
@oliviertassinari

oliviertassinari Dec 12, 2019

Author Member

@eps1lon What about we fallback to eval?

This comment has been minimized.

Sign in to view

Copy link
@eps1lon

eps1lon Dec 12, 2019

Member

And then @zloirock wants to transpile this as well because it doesn't work as native. First time I see actual hostile behavior by open sourcing something. Basically babel does not want to allow feature detection and rather over transpile.

Let's pin the babel plugins if these hostile changes are introduced and investigate if we should overhaul the logger import in the first place (e.g. transpile redux-logger)

This comment has been minimized.

Sign in to view

Copy link
@oliviertassinari

oliviertassinari Dec 12, 2019

Author Member

I don't think that it's "hostile", we are on the same boat, we are trying to navigate the scope of the options available.

I think that if a codebase makes the effort to check if assign is defined, it's probably fine to leave it alone.

This comment has been minimized.

Sign in to view

Copy link
@zloirock

zloirock Dec 12, 2019

Nothing hostile, the reason is here babel/babel#10859 (comment) - it should be detected as supported.
) {
// eslint-disable-next-line global-require
const createLogger = require('redux-logger').createLogger;
Expand Down

0 comments on commit 0de54dd

Please sign in to comment.