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

fix(build): missing babel helpers, true esm modules, simplify #290

Merged
merged 4 commits into from
Jun 20, 2018

Conversation

pascalduez
Copy link
Contributor

@pascalduez pascalduez commented May 28, 2018

Hi,

  • The Rollup Flow plugin is unnecessary, this is already handled by Babel and the babel-preset-flow.
  • The current esm build is still transformed to commonjs, it's actually built two times.
    So this simplify a bit things, and leverage Rollup for what it's good at (handling multiple builds).
  • The source files with Flow annotations should be output into the cjs dir, flow-bin is a node tool.
  • Don't exlude babel helpers from final bundle. Fixes Babel helpers missing #289 babelHelpers is not defined #294
    Only the typeof helper is used, so including it is amost costless. Including all helpers would,
    or requiring the users to install the plugin and including it in their code.

@vvo
Copy link
Contributor

vvo commented May 28, 2018

Nice work @pascalduez, notify us when ready to go

- This is already handled by babel-preset-flow
@ghost
Copy link

ghost commented Jun 18, 2018

There were the following issues with this Pull Request

  • Commit: 25e86e5
    • ✖ message may not be empty
      , - ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@pascalduez
Copy link
Contributor Author

@vvo I think it's good to go.

@pascalduez pascalduez changed the title WIP: Fix build, simplify config Fix build, simplify config Jun 18, 2018
@pascalduez pascalduez changed the title Fix build, simplify config fix(build) missing babel helpers, true esm modules, simplify Jun 18, 2018
@pascalduez pascalduez changed the title fix(build) missing babel helpers, true esm modules, simplify fix(build): missing babel helpers, true esm modules, simplify Jun 18, 2018
- Use Rollup for both cjs and esm build
- Don't re-run babel on top preventing overrides
- Since only the typeof helper is used, it's much more simple and
efficient to include it.
@ArtieReus
Copy link

This fix works for me. The given example from #294

reactElementToJSXString(<p style={{ color: 'red' }}>Test</p>)

goes through without errors.

@vvo vvo merged commit faa8f46 into algolia:master Jun 20, 2018
- yarn run smoke cjs default
- yarn run smoke esm default
- yarn run smoke cjs latest
- yarn run smoke esm latest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants