Skip to content

chore: add export maps to support native ESM #69

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

Merged
merged 1 commit into from
Dec 21, 2022

Conversation

layershifter
Copy link
Contributor

@layershifter layershifter commented Dec 21, 2022

What:

Fixes #68.

Why:

We would like to use /core as an entrypoint in our code, currently it impossible as rtl-css-js/core is a directory and we will get ERR_UNSUPPORTED_DIR_IMPORT from Node/bundlers 😨

How:

This PR adds import maps, I initially wanted to use import & require pair (see docs), but then I will have to change bundling process drastically and use something that emits .cjs/.js or .js/.mjs like tsup.

Why?!

  • type: "module" tells Node to have all .js as ESM, to use CJS - use .cjs files
  • type: "commonjs" tells Node to have all .js as CJS, to use ESM - use .mjs files
  • 💥
(node:23063) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
/node_modules/rtl-css-js/dist/esm/core.js:1
export { f as arrayToObject, h as calculateNewBackgroundPosition, j as calculateNewTranslate, i as canConvertValue, c as convert, d as convertProperty, k as flipSign, j as flipTransformSign, g as getPropertyDoppelganger, e as getValueDoppelganger, u as getValuesAsList, l as handleQuartetValues, m as includes, n as isBoolean, o as isFunction, r as isNullOrUndefined, q as isNumber, s as isObject, t as isString, a as propertiesToConvert, p as propertyValueConverters, b as propsToIgnore, w as splitShadow, v as valuesToConvert } from './convert-7f74cdb7.js';

So, I went with another option that I have seen in Babel's packages (reference):

  • don't set type (yeah, reference sets it, but this confuses Webpack)
  • use node (specific to Node env), import (for bundlers) & default
  • works in Node, works in Webpack ✅

Tests

Node / CJS

// index.js
console.log("typeof require('rtl-css-js')", typeof require('rtl-css-js'))
console.log("typeof require('rtl-css-js/core')", typeof require('rtl-css-js/core'))
console.log("typeof require('rtl-css-js/core.js')", typeof require('rtl-css-js/core.js'))
console.log("typeof require('rtl-css-js/package.json')", typeof require('rtl-css-js/package.json'))

⬇️⬇️⬇️

$ node index.js
typeof require('rtl-css-js') function
typeof require('rtl-css-js/core') object
typeof require('rtl-css-js/core.js') object
typeof require('rtl-css-js/package.json') object

Node, ESM

// index.mjs
console.log("typeof import('rtl-css-js')", typeof (await import('rtl-css-js')))
console.log("typeof import('rtl-css-js/core')", typeof (await import('rtl-css-js/core')))

⬇️⬇️⬇️

typeof import('rtl-css-js') object
typeof import('rtl-css-js/core') object

Webpack

// src/index.js
async function test() {
    console.log("typeof import('rtl-css-js')", typeof (await import('rtl-css-js')))
    console.log("typeof import('rtl-css-js/core')", typeof (await import('rtl-css-js/core')))
    console.log("typeof import('rtl-css-js/core.esm')", typeof (await import('rtl-css-js/core.esm')))
    console.log("typeof import('rtl-css-js/package.json')", typeof (await import('rtl-css-js/package.json')))
}

test().then(() => {})

⬇️⬇️⬇️

asset 346.js 7.22 KiB [compared for emit] [minimized] (id hint: vendors)
asset main.js 3.4 KiB [emitted] [minimized] (name: main)
asset 949.js 2.09 KiB [compared for emit] [minimized]
asset 503.js 707 bytes [compared for emit] [minimized]
asset 849.js 157 bytes [compared for emit] [minimized]
runtime modules 7.81 KiB 10 modules
cacheable modules 23.8 KiB
  modules by path ./node_modules/rtl-css-js/dist/esm/*.js 21.4 KiB
    ./node_modules/rtl-css-js/dist/esm/index.js 81 bytes [built] [code generated]
    ./node_modules/rtl-css-js/dist/esm/core.js 560 bytes [built] [code generated]
    ./node_modules/rtl-css-js/dist/esm/convert-7f74cdb7.js 20.8 KiB [built] [code generated]
  ./src/index.js 439 bytes [built] [code generated]
  ./node_modules/rtl-css-js/package.json 1.97 KiB [built] [code generated]

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value.
Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/

webpack 5.75.0 compiled with 1 warning in 245 ms

Checklist:

  • Documentation: N/A
  • Tests: Manually tested, with Webpack 5 & Node 16
  • Ready to be merged
  • Added myself to contributors table

"./core.js": {
"default": "./dist/cjs/core.js"
},
"./package.json": "./package.json"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added as it's useful to import it sometimes.

Comment on lines +22 to +30
"./core.esm": {
"default": "./dist/esm/core.js"
},
"./core.esm.js": {
"default": "./dist/esm/core.js"
},
"./core.js": {
"default": "./dist/cjs/core.js"
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Left for compat to avoid breaking changes.

@codecov
Copy link

codecov bot commented Dec 21, 2022

Codecov Report

Merging #69 (ece5766) into main (39e8b46) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #69   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          243       243           
  Branches        45        45           
=========================================
  Hits           243       243           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Very cool. I've never used the exports map before. Thanks for the help!

@kentcdodds kentcdodds merged commit 2dcf0d6 into kentcdodds:main Dec 21, 2022
@kentcdodds
Copy link
Owner

@all-contributors please add @layershifter for code

@allcontributors
Copy link
Contributor

@kentcdodds

@layershifter already contributed before to code

@kentcdodds
Copy link
Owner

🤦‍♂️

@layershifter layershifter deleted the chore/add-export-maps branch December 21, 2022 15:31
kentcdodds added a commit that referenced this pull request Dec 21, 2022
There was an issue with a patch release, so this manual-releases.md change is to release a new patch version.

Reference: #69
@github-actions
Copy link

🎉 This PR is included in version 1.16.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

usage of rtl-css-js/core throws in ESM packages
2 participants