Skip to content
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

Get rid of providesModule #6336

Closed
zpao opened this issue Mar 24, 2016 · 20 comments
Closed

Get rid of providesModule #6336

zpao opened this issue Mar 24, 2016 · 20 comments

Comments

@zpao
Copy link
Member

zpao commented Mar 24, 2016

It's been great for us at FB but not so much for everybody else. It's long been a goal to get rid of this and just use common js / es6 modules but haven't put the time in yet.

This change will require changes internally at FB and probably some work with fbjs so I'm going to assign this to myself.

@aweary
Copy link
Contributor

aweary commented Mar 24, 2016

Would this remove all the providesModule comments from the header comment in each file? I've found it really helpful when searching for a component in the repo (since there are no relative import paths).

@gaearon
Copy link
Collaborator

gaearon commented Mar 24, 2016

I've found it really helpful when searching for a component in the repo (since there are no relative import paths).

If we remove providesModule we’ll use relative paths like everybody else does.

@iamdustan
Copy link
Contributor

I’m curious if the “not so great for everyone else” bit is primarily a documentation/marketing issue? It’s business-as-usual for FB, but totally foreign and not referenced anywhere when someone comes looking at the code in here.

As a casual observation, Haste hasn’t seemed to have hindered outside contributors or their contributions to the RN project.

@aweary
Copy link
Contributor

aweary commented May 10, 2016

In the context of the discussion occurring in #6460, I want to start discussing how this might affect community tools that may rely on React internals. At enzyme we do import from the lib folder so we have a vested interest in how this might affect our ability to integrate.

@zpao
Copy link
Member Author

zpao commented May 10, 2016

Unknown until we do it. Ideally we would expose everything that a library like Enzyme would need. This impacts our addons-* packages as well.

@gaearon
Copy link
Collaborator

gaearon commented May 10, 2016

React.__SECRET_UTILS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED_UNLESS_YOU_ARE_ENZYME_IN_WHICH_CASE_ITS_FINE.

@acpower7
Copy link

How is this migration going?

@gaearon
Copy link
Collaborator

gaearon commented Jan 15, 2017

Nothing happening right now but I'm pretty sure we'll be looking at it this year since we want to compile React away to a flat bundle, and in this case Haste is not as useful to us. Moving the files easily is still nice so I'm not sure which approach we would take.

@acpower7
Copy link

acpower7 commented Jan 15, 2017

Thanks for the response ! One question, could you please further explain what you mean by compiling to a "flat bundle"? How is that different to what you guys are doing now?

@gaearon
Copy link
Collaborator

gaearon commented Jan 15, 2017

The idea is to always compile React into a single file, even on npm. We would use an optimized bundler that doesn't generate all the module registry code (like Rollup) so that the resulting bundle is smaller. It would also help solve some performance issues in the server environment where accessing process.env repeatedly is expensive.

@gaearon
Copy link
Collaborator

gaearon commented Apr 5, 2017

We now compile React into flat bundles (#9327) so this is likely one of the next steps.

@gaearon gaearon assigned trueadm and unassigned zpao Apr 5, 2017
@acpower7
Copy link

@gaearon Just to clarify, will this issue get rid of the haste system all together and use ES6 module syntax with relative paths?

@trueadm How soon will you get this done?

I'm looking forward to resolving this issue as I would like to explore/run this project with my fav IDE (Webstorm), which doesn't work with the current module system.

@gaearon
Copy link
Collaborator

gaearon commented Apr 14, 2017

We'll probably switch to ES modules first, and then get rid of haste. In a few weeks hopefully.

@swyxio
Copy link
Contributor

swyxio commented Sep 25, 2017

Hi, its not clear to me here.. is @providesModule removed from React but still alive in React Native?

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2017

I don’t think it’s removed in React either.

@swyxio
Copy link
Contributor

swyxio commented Sep 25, 2017

thanks Dan - but we should understand that the overall direction is to get rid of haste and therefore @providesmodule from React/RN? I like the extreme convenience but do see it as a potentially confusing pattern. So am a little confused on whether I should advocate its use for learners of React/RN.

@gaearon
Copy link
Collaborator

gaearon commented Sep 25, 2017

It shouldn't matter for learners: this is an internal convention. We are not advocating for others to use it. (And in fact, with changes in React 16, it will be impossible to require individual modules from React internals anyway.)

@gaearon
Copy link
Collaborator

gaearon commented Oct 25, 2017

👋
#11303

@gaearon gaearon closed this as completed Oct 25, 2017
@zen0wu
Copy link

zen0wu commented Nov 3, 2017

So this is removed in React with #11303, right? Are we going to do the same thing with react native? Currently it's impossible to require things like StyleSheet from react-native for this exact reason, in my understanding

@gaearon
Copy link
Collaborator

gaearon commented Nov 3, 2017

I don't know about React Native or about the problem you're describing. It might be better to ask in some RN community forum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants