Import from unified instead of relative index.js#217
Import from unified instead of relative index.js#217remcohaszing wants to merge 1 commit intomainfrom
Conversation
I don’t think this is really done this way? const mod = await import('unified')
assert.deepEqual(
Object.keys(mod).sort(),
['unified'],
'should expose the public api'
)If there are other exports, we can have such blocks for those too. Furthermore, doesn’t this actually checks the nested |
You’re right, I was under the impression we already used export maps. I think it’s nice to use them, because it marks internals as private. This prevents accidental use, I think also of internal types. But indeed, it’s a breaking change.
Given the above statement that we don’t use export maps, I think you mean this: const mod = await import('../index.js')
assert.deepEqual(
Object.keys(mod).sort(),
['unified'],
'should expose the public api'
)I don’t really see the value of this assertion TBH. We already test against the main Another benefit of my proposal (for packages that do use export maps) is that we can test against multiple export conditions. This is already done in https://github.com/syntax-tree/hast-util-from-html-isomorphic/blob/main/package.json#L64-L66 for example. node --test
node --test --conditions browser
node --test --conditions worker |
We don’t test what’s part of the API. It is good to test what is part of the API. |
Initial checklist
Description of changes
In test code, imports now import from
unifiedinstead of a relative path toindex.js. This way package exports are also tested, both runtime and for type checking. This truly tests the public interface.I’m not saying we should go over all packages to apply this immediately, but when working on a package, I think it’s a good idea to apply this pattern. This PR is mostly to share and discuss the idea.