New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thoughts on making react/lib/keyMirror an NPM module #1639

Closed
nhunzaker opened this Issue Jun 4, 2014 · 18 comments

Comments

Projects
None yet
@nhunzaker
Collaborator

nhunzaker commented Jun 4, 2014

I'm finding I really love something like keyMirror on non-react related fluxy projects. Is there a good NPM module for this, or considerations of making it a separate module?

@STRML

This comment has been minimized.

Show comment
Hide comment
@STRML

STRML Jun 4, 2014

Contributor

I thought the same thing, so I published keyMirror as a standalone module.

Contributor

STRML commented Jun 4, 2014

I thought the same thing, so I published keyMirror as a standalone module.

@nhunzaker

This comment has been minimized.

Show comment
Hide comment
@nhunzaker

nhunzaker Jun 4, 2014

Collaborator

Awesome! I heard some discussion about splitting out some utils in React into separate modules like this. I'm curious to know if Facebook would consider using @STRML's module, or if they'd rather keep things under their control.

If not, I'd be happy to send out a PR.

Collaborator

nhunzaker commented Jun 4, 2014

Awesome! I heard some discussion about splitting out some utils in React into separate modules like this. I'm curious to know if Facebook would consider using @STRML's module, or if they'd rather keep things under their control.

If not, I'd be happy to send out a PR.

@STRML

This comment has been minimized.

Show comment
Hide comment
@STRML

STRML Jun 4, 2014

Contributor

It's a super simple module - I think, in the end, it would be better to separate react/lib into react-lib, a module with no dependencies, and allow users to require a single one at a time, e.g. react-lib/keyMirror. Smart packagers, e.g. webpack/browserify, would pack this efficiently, FB wouldn't have to publish a ton of separate modules, and it could be simply required from within react itself.

Contributor

STRML commented Jun 4, 2014

It's a super simple module - I think, in the end, it would be better to separate react/lib into react-lib, a module with no dependencies, and allow users to require a single one at a time, e.g. react-lib/keyMirror. Smart packagers, e.g. webpack/browserify, would pack this efficiently, FB wouldn't have to publish a ton of separate modules, and it could be simply required from within react itself.

@nhunzaker

This comment has been minimized.

Show comment
Hide comment
@nhunzaker

nhunzaker Jun 4, 2014

Collaborator

👍

Collaborator

nhunzaker commented Jun 4, 2014

👍

1 similar comment
@tommymarshall

This comment has been minimized.

Show comment
Hide comment
@tommymarshall

tommymarshall commented Jun 4, 2014

👍

@petehunt

This comment has been minimized.

Show comment
Hide comment
@petehunt

petehunt Jun 4, 2014

Contributor

We want to do this, but versioning will be an interesting problem which is why we haven't done it yet (these are common core modules which may need to be versioned separately from React for e.g. jest). Stay tuned...

Contributor

petehunt commented Jun 4, 2014

We want to do this, but versioning will be an interesting problem which is why we haven't done it yet (these are common core modules which may need to be versioned separately from React for e.g. jest). Stay tuned...

@jordwalke

This comment has been minimized.

Show comment
Hide comment
@jordwalke

jordwalke Jun 4, 2014

Member

👍

@petehunt What's the challenge here? Most of these won't change a bunch.

Member

jordwalke commented Jun 4, 2014

👍

@petehunt What's the challenge here? Most of these won't change a bunch.

@ToucheSir

This comment has been minimized.

Show comment
Hide comment
@ToucheSir

ToucheSir Jun 5, 2014

Out of pure curiosity, why does keyMirror take an object instead of an array of "enum" values? Is there a solid use-case for passing an object with null keys over an array? (under the impression I'm missing some vital point)

ToucheSir commented Jun 5, 2014

Out of pure curiosity, why does keyMirror take an object instead of an array of "enum" values? Is there a solid use-case for passing an object with null keys over an array? (under the impression I'm missing some vital point)

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Jun 5, 2014

Member

@ToucheSir The main purpose of keyMirror is to deal with the fact that Closure Compiler advanced mode crushes keys, which allows you to write code like

keyMirror({monkey: null, gorilla: null})

and have it become something like

k({m:null,g:null})

which evaluates to

{m:"m",g:"g"}

at runtime. If it was specified as a list of strings, they wouldn't get crushed matching the property names.

Member

sophiebits commented Jun 5, 2014

@ToucheSir The main purpose of keyMirror is to deal with the fact that Closure Compiler advanced mode crushes keys, which allows you to write code like

keyMirror({monkey: null, gorilla: null})

and have it become something like

k({m:null,g:null})

which evaluates to

{m:"m",g:"g"}

at runtime. If it was specified as a list of strings, they wouldn't get crushed matching the property names.

@petehunt

This comment has been minimized.

Show comment
Hide comment
@petehunt

petehunt Jun 5, 2014

Contributor

@jordwalke and what happens when they do? what if we release a component that depends on a version of merge() that conflicts with another component used in the app? Even worse, what if the module were stateful?

BTW, we were talking about changing merge() on some thread yesterday, so this is not a super long shot. Versioning react, its dependencies and dependents is a difficult problem, especially with FB's internal development process and our desire to dogfood everything in open source.

Contributor

petehunt commented Jun 5, 2014

@jordwalke and what happens when they do? what if we release a component that depends on a version of merge() that conflicts with another component used in the app? Even worse, what if the module were stateful?

BTW, we were talking about changing merge() on some thread yesterday, so this is not a super long shot. Versioning react, its dependencies and dependents is a difficult problem, especially with FB's internal development process and our desire to dogfood everything in open source.

@ToucheSir

This comment has been minimized.

Show comment
Hide comment
@ToucheSir

ToucheSir Jun 5, 2014

@spicyj Thanks for the insight! Didn't even bother to think down that route, but now keyMirror is gaining appeal by the second. :)

ToucheSir commented Jun 5, 2014

@spicyj Thanks for the insight! Didn't even bother to think down that route, but now keyMirror is gaining appeal by the second. :)

@raminbp

This comment has been minimized.

Show comment
Hide comment
@raminbp

raminbp Sep 14, 2015

@spicyj i didn't got it, can you explain it more ? what's the whole point of the keymirror ?

raminbp commented Sep 14, 2015

@spicyj i didn't got it, can you explain it more ? what's the whole point of the keymirror ?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Sep 14, 2015

Member

i didn't got it, can you explain it more ? what's the whole point of the keymirror ?

Google Closure Compiler has an advanced mode. It's more powerful than UglifyJS and will compress your code like crazy. In advanced mode, any property names you define will be “crushed” (unless you're defining them as strings).

When you have code like

var Stuff = {
  GOOD_STUFF: 'GOOD_STUFF'
};

Closure Compiler in advanced mode will compress it to

var a = {
  b: 'GOOD_STUFF'
};

which will break your code.

There are two options:

a) Use string property name to opt out of crushing

var a = {
  'GOOD_STUFF': 'GOOD_STUFF'
};

This works, but you miss out on a nice optimization opportunity.

b) Use a helper that makes sure keys and values match

That's what keyMirror does.

Member

gaearon commented Sep 14, 2015

i didn't got it, can you explain it more ? what's the whole point of the keymirror ?

Google Closure Compiler has an advanced mode. It's more powerful than UglifyJS and will compress your code like crazy. In advanced mode, any property names you define will be “crushed” (unless you're defining them as strings).

When you have code like

var Stuff = {
  GOOD_STUFF: 'GOOD_STUFF'
};

Closure Compiler in advanced mode will compress it to

var a = {
  b: 'GOOD_STUFF'
};

which will break your code.

There are two options:

a) Use string property name to opt out of crushing

var a = {
  'GOOD_STUFF': 'GOOD_STUFF'
};

This works, but you miss out on a nice optimization opportunity.

b) Use a helper that makes sure keys and values match

That's what keyMirror does.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Sep 14, 2015

Member

Random usage of fbjs isn't really supported but we moved keyMirror out of React to there (require('fbjs/lib/keyMirror'). Closing out.

Member

zpao commented Sep 14, 2015

Random usage of fbjs isn't really supported but we moved keyMirror out of React to there (require('fbjs/lib/keyMirror'). Closing out.

@zpao zpao closed this Sep 14, 2015

@israelito3000

This comment has been minimized.

Show comment
Hide comment
@israelito3000

israelito3000 Nov 14, 2015

Hello, probably this sound redundant, I am using react by the bower package and I am trying to use keyMirror which is inside React.js, but this is not exposed as a global object, how can I use it without import another keyMirror package?

israelito3000 commented Nov 14, 2015

Hello, probably this sound redundant, I am using react by the bower package and I am trying to use keyMirror which is inside React.js, but this is not exposed as a global object, how can I use it without import another keyMirror package?

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Nov 14, 2015

Member

You can't.

Member

sophiebits commented Nov 14, 2015

You can't.

@codejamninja

This comment has been minimized.

Show comment
Hide comment
@codejamninja

codejamninja May 15, 2018

Wouldn't it be easier to use key mirror the following way

keyMirror(['monkey', 'gorilla']);

Instead of

keyMirror({monkey: null, gorilla: null});

codejamninja commented May 15, 2018

Wouldn't it be easier to use key mirror the following way

keyMirror(['monkey', 'gorilla']);

Instead of

keyMirror({monkey: null, gorilla: null});
@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits May 15, 2018

Member

See my comment on June 5 for the answer to your question.

keyMirror isn't and has never been a supported API of React. I'm going to lock this conversation to reduce noise.

Member

sophiebits commented May 15, 2018

See my comment on June 5 for the answer to your question.

keyMirror isn't and has never been a supported API of React. I'm going to lock this conversation to reduce noise.

@facebook facebook locked as off topic and limited conversation to collaborators May 15, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.