Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

ESM build, misc other tweaks #968

Merged
merged 42 commits into from
Feb 9, 2018
Merged

ESM build, misc other tweaks #968

merged 42 commits into from
Feb 9, 2018

Conversation

ryan-roemer
Copy link
Member

@ryan-roemer ryan-roemer commented Feb 7, 2018

This builds on @kale-stew 's #967 and goes further down the rabbit hole.

Working Items

  • babel-preset-add-module-exports does not seem to work in producing module.exports = exports["default"] in e.g. lib/index.js

  • Confusingly, the plugin is needed for our frontend Karma test stuff and seems to work there! 😕 And we have mocked stuff like

    const resolveStyles = require('inject-loader!resolve-styles')({
      exenv: require('__mocks__/exenv')
    });

    that plays poorly with removing babel-preset-add-module-exports (I tried replacing all require("foo") with require("foo").default)

Tasks

@kale-stew --

  • Look for all instances of "server" or "SSR" or "Node" or "Node.js", etc. and replace require('radium') with require('radium').default.
  • Places to look: README.md, docs/**, examples/**...
  • Look for any remaing require()s and convert client code / babel'ed code examples to import Radium from 'radium'; or other analogous thing.

@ryan-roemer --

  • Verify dist works as expected with global.Radium variable in prod + dev modes.
  • Get yarn run test-frontend to pass.
  • Check CI passes...
  • Try to remove all legacy things like Radium.PROPERTY_NAME =

Done

  • Figure out if keeping babel-preset-add-module-exports
  • Figure out if breaking API with removing module.exports thing
  • Checking if node4 works in Travis and Appveyor.

Changes

  • Output es/ directory with ESM export/imports intact. Closes Add es build to Radium. #966
  • Updates to babel-preset-env
  • Convert Node.js server example to use require()s
  • Use builder concurrent to parallelize a lot of our build / test stuff
  • Remove {__clearStateForTests, __setTestMode} as named ESM exports because they're not. Switch to just using resolveStyles object directly.
  • Remove package-lock.json (we'll stick with yarn)

/cc @alexlande @kale-stew

@coveralls
Copy link

coveralls commented Feb 7, 2018

Coverage Status

Coverage remained the same at 94.406% when pulling ac9301b on task/es-build-ryan into 33fe30e on master.

@ryan-roemer
Copy link
Member Author

The Problem

We do rely on the babel-preset-add-module-exports plugin to convert:

export default Radium;

to:

exports.default = Radium;
module.exports = exports['default']; // plugin adds this

for our legacy compatibility. However, if we have any other named export, like:

// ESM re-exports
export {Plugins, Style, StyleRoot, getState, keyframes};

That I added, the module.exports = exports['default']; line is not added. This explains why the demo breaks / it's not in babel source but the tests both need the plugin and pass (because they rely on it in other files).

As far as I can tell, only src/index.js is affected by this and nothing else apparently has a combination of named + default exports (that are in any way relied on).

You can require `Radium` on the server / using CommonJS with:

```jsx
var Radium = require('radium');

Choose a reason for hiding this comment

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

Do we not need .default on this import?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. That is the change with the new root index.js file.

Choose a reason for hiding this comment

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

Got it. 👍

@ryan-roemer
Copy link
Member Author

@alexlande @exogen -- I've got this branch and a separate experiment repo for all of the consumption scenarios I could think of. Can you take a look at that (and this PR) and let me know if we're missing anything or anything needs changing for this approach / PR?

https://github.com/FormidableLabs/radium-experiments-v0.22

Thanks!!!

@ryan-roemer ryan-roemer changed the title WIP: ESM build, misc other tweaks ESM build, misc other tweaks Feb 8, 2018
@alexlande
Copy link
Contributor

Went through the experiments, everything looks good as far as I can tell. Thanks for all of the work here, module system interop is so complicated 👍

@ryan-roemer ryan-roemer merged commit 0001fcb into master Feb 9, 2018
@ryan-roemer ryan-roemer deleted the task/es-build-ryan branch February 9, 2018 21:09
@ryan-roemer
Copy link
Member Author

Released in radium@0.22.0 🎉

jaredbrookswhite pushed a commit to jaredbrookswhite/radium that referenced this pull request Aug 22, 2018
- **Breaking Change**: Radium now exports defaults as `.default`, so for runtimes like Node.js for all files in `lib/**`. We have changed `package.json:main` to point to `/index.js` instead of `/lib/index.js` as a convenience wrapper to expose mostly what was there before so behavior of `const Radium = require('radium');` works mostly as it did before. Caveats:
- Add `es` ESM module export files.
- Fix `package.json:scrtips.postinstall` task to correctly work for git-based dependencies.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add es build to Radium.
4 participants