Skip to content
This repository has been archived by the owner on Jan 29, 2023. It is now read-only.

Use Babel for builds #31

Closed
SachsKaylee opened this issue Jul 30, 2018 · 10 comments
Closed

Use Babel for builds #31

SachsKaylee opened this issue Jul 30, 2018 · 10 comments

Comments

@SachsKaylee
Copy link
Collaborator

SachsKaylee commented Jul 30, 2018

Webpack is meant for the final bundling of a code base. Libraries are better built using raw Babel. This allows the user to use their own module bundler/polyfills/etc...

This entails upgrading to the latest Babel version and removing webpack as a dev-dependency for the library (we will still use it for the example).

@SachsKaylee SachsKaylee added this to the Streamline milestone Jul 30, 2018
@SachsKaylee SachsKaylee self-assigned this Jul 30, 2018
@SachsKaylee
Copy link
Collaborator Author

SachsKaylee commented Jul 30, 2018

Hey, @AndrewRedican

should you have some time can you review the updates to the build system? It is pretty much a complete redo, so I'll try to explain the changes as much as possible.

See the babel-builds branch.

First of all is the move from webpack to babel for building the library.

Why? While webpack is an awesome tool, it is in my experience not the greatest tool for compiling libraries: The library still compiles to a single bundle.js, which is not desired for them(Always includes all code even if they don't need all of it, they can't properly apply their own transformers/source maps/etc to it, ...)

With the babel backed compilation we retain the same folder/file structure, allowing users to manually import parts of the library, while still transpiling the module to plain commonjs es2015.

We currently achieve this by using the WebpackCopyPlugin, but this solution doesn't allow us to transform the code and is clearly not scalable as the project will evolve.

package.json scripts

There are a decent amount of new package.json scripts. Let's go though them:

  • contributors:add - As before
  • contributors:generate - As before
  • test - As before
  • test:watch - As before
  • test:coverage - As before
  • test:release - Creates a test release build containing all files that would be published to NPM, can be installed in another project to test how the published library would behave. (Forces a full build beforehand)
  • build - Builds the entire library by running the four build scripts listed below in the listed order.
  • build:es2015 - Transpiles the library to es2015 and puts the output in /dist
  • build:es2015modules - Transpiles the es2015 output to use the commonjs module system (used by node.js), overwrites previous output
  • build:es - Transpiles the library to plain es using the latest features. This means only things like React will be transformed. Placed in /dist/es
  • build:copy-files - Copies README, CHANGELOG, LICENSE & package.json to /dist and prepends some license information to index.js
  • release - Publishes the /dist(!!) folder to NPM (Forces a full build beforehand)

Problems?

I still have to wrap my head around jest and unit testing in general, so it's very likely that the tests fail at the moment. - I got it to work to a point where it starts and doesn't fail due to syntax errors, but fails due to an error about localStorage in jsdom. I'll look into that tomorrow.

This is a breaking change for users. The previous import paths were e.g. react-json-editor-ajrm/dist/locale/en. Since we now directly publish the dist folder, the new import paths are react-json-editor-ajrm/locale/en. We could obviously change that, but I think this is the better way to move forward.

@AndrewRedican
Copy link
Owner

AndrewRedican commented Jul 30, 2018

@PatrickSachs,

🔥 This is great! I think this is the correct approach for bundling the project. I too noticed it was very difficult to create multiple bundles in Webpack (I tried setting up configuration for multiple bundles when I realized the react-json-editor-ajrm/locale/en import was not working as expected with the distribution version.

This is a breaking change for users. The previous import paths were e.g. react-json-editor-ajrm/dist/locale/en. Since we now directly publish the dist folder, the new import paths are react-json-editor-ajrm/locale/en. We could obviously change that, but I think this is the better way to move forward.

I too think react-json-editor-ajrm/locale/en is a lot cleaner.

build:es - Transpiles the library to plain es using the latest features. This means only things like React will be transformed. Placed in /dist/es

Can you expand on this? I didn't quite follow.

I think this update clears the biggest concerns of the Streamline milestone. Please give me few hours to get familiar with this new configuration, I'll check tests.

@SachsKaylee
Copy link
Collaborator Author

SachsKaylee commented Jul 30, 2018

build:es - Transpiles the library to plain es using the latest features. This means only things like React will be transformed. Placed in /dist/es

Can you expand on this? I didn't quite follow.

Certainly - Browser support for ES6+ is getting better and better. Some browsers like the Safari 10, Chrome 61, ... actually already have native support for the ES6 module ecosystem(import/export).

The es compile script allows us to transpile code that is not part of actual ES6(like React JSX and some proposals) into valid ECMAScript 6 using the latest language features.

This code can then be consumed either by modern runtimes(like the mentioned Safari, or node.js with the right flags), or used by the build system of other projects to transpile & polyfill the code the way they want to.

Since this build system is heavily based on the way material-ui has set theirs up, see this PR in material-ui for some discussion about the merits of having an ES6 output: mui/material-ui#8772

If it seems like too much overhead for now(which I can understand, I was initially rather skeptic myself) we could remove the es configuration and just compile to a single es5 output. The benefit of this would be simplicity and reduced build time.

@SachsKaylee
Copy link
Collaborator Author

I thought about this system today, and I think having a simpler build system is better and less confusing.

We still have two different build outputs, but apply the same babel transformers to all of them. The only difference is that one output's module system is commonjs (export, require - something that node.js can use), and the other is es (export, import - something browsers can use). Most bundlers should understand both module systems.

  • build - Same as before
  • build:commonjs - Builds the library for common JS modules - /dist
  • build:es - Builds the library for ES modules - /dist/es
  • build:copy-files - Same as before

One downside of this is that we transpile both setups to ES versions compatible with the browsers specified in the babel.config.js even if the user perhaps only targets newer ones. We thus potentially include unneeded polyfills/transformations.

Let me know if you agree with this change or want to go back to the old configuration.

@AndrewRedican
Copy link
Owner

Hey @PatrickSachs,

Sorry I haven't been able to get back to you. I haven't had the time to finish reviewing. I'll confirm today.

That being said, I am happy to provide two different versions for distribution:

  1. One that has been transpiled down to the simplest version of javascript (which may include additional polyfills/transformations and bigger bundle size). If possible something that could be run by older browsers even, like IE.
  2. Another with little to no transformations at all to let other developers make the transformation themselves.

I don't mind the "complicated" setup. I don't mind longer build times either. As long as we are providing the flexible solutions for other developers that would like to use the project, I think it is a fair trade.

Do you think the new set up proposed is scalable? For example, would we have trouble adding more files in the locale and possible another folder for syntax? If the answer is it is unlikely to have problems, we can stick with the new project setup.

@SachsKaylee
Copy link
Collaborator Author

Sorry I haven't been able to get back to you. I haven't had the time to finish reviewing. I'll confirm today.

No worries! Take your time, it's important to set up a good and scalable compiler.

One that has been transpiled down to the simplest version of javascript (which may include additional polyfills/transformations and bigger bundle size). If possible something that could be run by older browsers even, like IE.

As of now all versions are compiled to IE11 compatible code. This has to be battle tested, but that's what the configuration tells Babel to do.

targets: {
  ie: 11,
  edge: 14,
  firefox: 45,
  chrome: 49,
  safari: 10,
  node: '6.11',
}

We might need a plugin explicitly for polyfills in the future. (To avoid problems like #26)

Another with little to no transformations at all to let other developers make the transformation themselves.

So, have the commonjs Version be the one compiled down to "IE Level"-Code, and the ES one be modern JavaScript? (Essentially somewhat like before)

For example, would we have trouble adding more files in the locale and possible another folder for syntax

No, the setup will transpile the folder structure 1:1, so any directory or file added in /src will be included in the transpiled /dist output.

@SachsKaylee
Copy link
Collaborator Author

Alright, I'm rather statisfied with this change: As discussed, the commonjs version will be transpiled & polyfilled to something that should be compatible with older browsers, while the ES version stays largely untouched(aside from transpiling react and language proposals) allowing the user to decide how to further compile the code.

I'll go ahead and create a PR for this change. Let me know if run into any issues during your test or have additional suggestions/requests for changes.

@AndrewRedican
Copy link
Owner

I'll be adding new base tests. I'll also look into browser compatability testing. I'll be using this

@AndrewRedican
Copy link
Owner

#33

@AndrewRedican
Copy link
Owner

This task is complete

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

No branches or pull requests

2 participants