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

Element: Refactor element package to be React Native friendly #7740

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@gziolo
Member

gziolo commented Jul 6, 2018

Description

This PR refactors @wordpress/element package to follow the structure we use in other packages. In particular, I broke down the code to be located in the following folders:

  • api - all the api methods from React and React DOM (including Fragment and StrictMode from React which might be considered components)
  • components - our custom components

This enforced also moving tests to new files, as well as updating all local references.

This change will help to maintain a version of the package which works for both web and React Native:

  • It gives more granular control over what we override for RN.
  • It removes the highly undocumented react-native entry point and uses a more robust solution based on the main entry point.

NPM package

This will temporarily break npm package if we publish it to npm, similar to what we observed in other packages using export * from ... because Babel has this bug which adds:

import _Object$defineProperty from 'babel-runtime/core-js/object/define-property';
import _Object$keys from 'babel-runtime/core-js/object/keys';

How has this been tested?

npm run dev
npm run build
npm test
npm run test-e2e

Types of changes

Refactoring only, there is no change in logic.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@gziolo gziolo self-assigned this Jul 6, 2018

@gziolo gziolo requested review from hypest and WordPress/gutenberg-core Jul 6, 2018

@gziolo gziolo added this to In Progress in API freeze via automation Jul 10, 2018

@gziolo gziolo removed this from In Progress in API freeze Jul 10, 2018

@aduth

Honestly, I'd rather we just split the files out within the same top-level folder. I don't want to encourage people to think it's fine to add components to this module. It should be an exceedingly rare use case.

@gziolo

This comment has been minimized.

Member

gziolo commented Jul 16, 2018

Honestly, I'd rather we just split the files out within the same top-level folder. I don't want to encourage people to think it's fine to add components to this module. It should be an exceedingly rare use case.

I will try to open another PR and follow your recommendation. We should have also less code to move around since we extracted @wordpress/compose package.

@gziolo gziolo closed this Jul 16, 2018

@gziolo gziolo deleted the update/element-rn-friendly branch Jul 16, 2018

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