feat: Add CommonJS export for parse5 module #418
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,15 @@ | |
| "serialize" | ||
| ], | ||
| "license": "MIT", | ||
| "main": "dist/cjs/index.js", | ||
|
There was a problem hiding this comment. wouldn't we still want the default to be ESM? so the only way to reach any commonjs entrypoints is by reading the package exports There was a problem hiding this comment. Node versions with ESM support will not read the |
||
| "module": "dist/index.js", | ||
| "types": "dist/index.d.ts", | ||
| "exports": { | ||
| ".": { | ||
| "import": "dist/index.js", | ||
| "require": "dist/cjs/index.js" | ||
| } | ||
| }, | ||
|
Comment on lines
+33
to
+38
There was a problem hiding this comment. this breaks rehype/test/parse-error.js in rehypejs/rehype#113 import p5errors from 'parse5/lib/common/error-codes.js'
wontfix in rehype, as ERR is not exported by parse5 → export as ErrorCodes? https://nodejs.org/api/esm.html
There was a problem hiding this comment. the package exports are right IMO. if we have consumers of the error codes enum, we should probably just export it top-level along with i.e. in |
||
| "repository": { | ||
| "type": "git", | ||
| "url": "git://github.com/inikulin/parse5.git" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "extends": "./tsconfig.json", | ||
| "compilerOptions": { | ||
| "module": "CommonJS", | ||
| "target": "ES6", | ||
| "outDir": "dist/cjs" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this make node think
dist/cjs/is its own package?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will make
require('parse5')work in Node versions with ESM support.Because the files are plain
.jsfiles, Node will look at the closestpackage.jsonfile to determine if the code is ESM or not. We create apackage.jsonfor the CJS code, to make sure requiring it works as expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i get that part, was just wondering what other effects it might have. since you're telling node the
cjspath is its own standalone package by giving it its own package manifest. curiosity more than anything, just wondering what other behaviour it might change in tooling.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of any side-effects, but definitely something worth keeping in mind for future bug-reports. I adopted the pattern from this article from SenseDeep, which is used in several published (but not very popular) modules.