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

Yarn Workspaces Follow-up #11275

Closed
13 of 19 tasks
gaearon opened this issue Oct 18, 2017 · 1 comment
Closed
13 of 19 tasks

Yarn Workspaces Follow-up #11275

gaearon opened this issue Oct 18, 2017 · 1 comment

Comments

@gaearon
Copy link
Collaborator

gaearon commented Oct 18, 2017

Should Do Soon

  • Having a package called react-native is weird.

    Maybe we can just rename it to react-native-renderer to clarify the difference.

  • Better strategy for shared code?

    Currently we just pile code into shared without particular structure, and then import directly through Haste. This is the same as we do in master though, so it’s at least no worse. We might want to tighten that up a little bit in the future, and maybe have some first-class entry points into it. We need to be clear shared code will be duplicated between any renderers using those modules.

  • Server shouldn't depend on reconciler modules.

    I noticed warnValidStyle depends on ReactDebugCurrentFiber. This seems like it will always miss owner in warnings on server. Maybe it should depend on some other shared file instead? Regardless, this is already an issue on master so it’s not a regression.

  • Find a more solid way to avoid Jest skipping our own modules (current hack is gross).

    The hack being "transformIgnorePatterns": ["/node_modules/(?!react)"], in package.json. Otherwise Jest skips our own code thinking it doesn’t need to be compiled because it was resolved through node_modules.

    The hack is not dangerous and can’t cause false positives with third-party code because I specifically check the resolved symlink path in the Jest transformer. Still, it means that if we add a package whose name doesn’t start with react, it will not be Babel-ed in tests until we add another exceptional case there.

    We can live with this hack for a little bit since we don’t plan to publish packages that don’t start with react in near future. We should file an issue with Jest though and come up with a better solution.

    Update: fixed via Update Jest and remove hacks #11372.

Should Do Later

  • Get rid of useFiber flag in bundle config
  • Rename entry points to be more intuitive (e.g. React should be React entry point etc)
  • Make internal folder structure more sensible
  • Move requires around to be prettier
  • Fix relative requires in tests
  • Get rid of fbEntry in bundle config
  • Why are we bundling lowPriorityWarning into PROD bundle?
  • In general, remove anything in bundle config that can now be determined using package folder
  • Fix the gross hack Correctly replace shims using relative requires #11472
  • Forbid importing from other packages? Drop Haste #11303 (comment)
  • Move some Flow types
  • It is odd that package-level deps must be at top level too Make "art" a dependency of "react-art" #11341 (comment)
  • Split bundle config per package and "autodiscover" packages for bundling and version check
  • Autogenerate "simple" entry point files for npm (react-dom/npm/index.js being the exception)
  • Move some of scripts into packages too?
@gaearon gaearon self-assigned this Oct 18, 2017
@gaearon gaearon mentioned this issue Oct 24, 2017
10 tasks
@gaearon
Copy link
Collaborator Author

gaearon commented Jan 2, 2018

Meh on other things in the checklist. If they come up again we might do it.

@gaearon gaearon closed this as completed Jan 2, 2018
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

1 participant