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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update build tooling and exports #1869

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

tux-tn
Copy link
Member

@tux-tn tux-tn commented Nov 16, 2021

Disclaimer: This is a big update

Mainly what have been done:

  1. Update and use rollup for all builds (CJS, UMD and ES6)

We had multiple commands for building for browser, node and es6. Since we are already using rollup, i merged all needed bundles in one single config file and command to allow easier maintenance over time.

  • Now the babel config is only needed by mocha, we are passing separate babel options for each bundle type.
  • Added a new target ES6 for browsers that generates minified mjs files (Not sure if we should keep two es6 exports or just the mjs one, may be @fedeci can provide some feedback on this) targeting modern browsers (Browsers and versions of NodeJS that support JS imports to be more specific)
  • Replaced uglifyjs by terser in order to target modern JS when minifying ES6 for browsers.
  • Moved from 9 cleaning/building commands to 3 in package.json .
  1. Updates to eslint
  1. Bump minimum node version for tests
  • Dropped CI tests for unmaintained NodeJS versions. The need to run tests for NodeJS 6 should be replaced with browser tests. (CJS and UMD target is still the same aka NodeJS 0.10).
  1. Add es6 exports with tree shaking

One of the big problems of our ES6 exports is that we are not allowing tree-shaking as we can see in #1593 #1759 #683 and many more issues.

  • A new file index.es.js has been added in order to allow tree-shaking for es6 exports. Now we can hopefully import validators in multiples ways:
import * as validator from 'validator';

import {isEmail, isUrl } from 'validator';

// The old behaviour is kept and the imports below also work:

import validator from 'validator';

import isEmail from 'validator/es/isEmail';
  • The readme has been updated in order to reflect the added use cases and also to provide examples of JS modules imports in browsers from unpkg and jsDelivr

Other changes:

  • Dropped bower config and mention of it in the readme since the project stopped many years ago
  • ES6 exports use different browser target than CJS and UMD. That way we can still support old NodeJS versions without having unnecessary polyfills.
  • Bumped the copyright year in the readme and license file (We didn't do it since 2018 馃槄 )
  • Added source, module and browser entries to package.json` (Not sure if we should keep source/browser since they are only giving hints)
  • Updated mocha config to allow test results to be more verbose (From dots to text) in order to allow contributors to find more easily the failing tests and reverted an old change I've made for coverage reporter from cobertura to lcov

What need to be done:

  • Add browser tests and CI Job using Browserstack
  • Add specific tests for tree shaking and JS modules imports
  • Update the PR with more details about each added feature

- Bump build dependencies
- Use rollup for all outputs to reduce building time
- Transpile to modern es2017 code for browsers supporting es modules using `@babel/preset-modules`
- Give hints to javascript bundlers about source for es modules and browser script to use
- Migrate from uglifyjs to terser (Allow minifying for modern browsers)
We are in 2021, bower was deprecated a long time ago
@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #1869 (5de0bd3) into master (cfcf911) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master     #1869    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          103       103            
  Lines         2097      1949   -148     
  Branches       473         0   -473     
==========================================
- Hits          2097      1949   -148     
Impacted Files Coverage 螖
src/lib/isIP.js 100.00% <酶> (酶)
src/lib/isRFC3339.js 100.00% <酶> (酶)
src/index.js 100.00% <100.00%> (酶)
src/lib/isCurrency.js 100.00% <100.00%> (酶)
src/lib/isIdentityCard.js 100.00% <100.00%> (酶)
src/lib/isIn.js 100.00% <100.00%> (酶)
src/lib/isLicensePlate.js 100.00% <100.00%> (酶)
src/lib/isMobilePhone.js 100.00% <100.00%> (酶)
src/lib/isPostalCode.js 100.00% <100.00%> (酶)
... and 87 more

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update cfcf911...5de0bd3. Read the comment docs.

@pixelbucket-dev
Copy link

@tux-tn is there a way breaking out the version bumps into a separate PR and merging that separately? That should be high-prio because of #2123. If that is not possible, what is holding back this PR? Thanks :).

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

Successfully merging this pull request may close these issues.

None yet

2 participants