Fix types for NodeNext #2365
Fix types for NodeNext #2365
Conversation
|
@epoberezkin I think this is ready for review now, and I'm now just hitting the node 14 build issue plaguing the main branch. Maybe we can disable now that node 14 is EOL? |
This reverts commit b7b5fd0.
|
Okay yep this is ready. You can see the build passing on the other builds in b7b5fd0, where the only change I did was temporarily remove 14 from the matrix. |
| import * as runtimeValidationError from "./runtime/validation_error" | ||
| import * as compileRefError from "./compile/ref_error" | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-namespace, no-redeclare |
There was a problem hiding this comment.
seems typescript-eslint isn't very aware of the export import feature in namespaces typescript-eslint/typescript-eslint#4129
benasher44
force-pushed
the
basher/ajv-export-fix
branch
from
9c0c9ba to
e935ed9
Compare
benasher44
force-pushed
the
basher/ajv-export-fix
branch
from
e935ed9 to
804d27a
Compare
This reverts commit c0d9645.
|
Pushed more updates + confirmed the build pass without node 14 |
|
Thanks @jasoniangreen! Let me know if you have feedback on this PR — would love to move this forward |
Let me update your branch with master and have a look. |
|
Something that hasn't been considered with this change is the browser bundles we generate. With this PR the rollup command, which is used to generate browser bundles, doesn't work. Either way we have to be wary of the various exports and ways that AJV is used and ensure we don't make life difficult for a lot of people by changing things unnecessarily. |
|
That's totally fair! Would you consider just adding named exports for the classes alongside the default exports? The main pain point is the default exports. If people have the named exports as an alternative, I think that would satisfy most folks, even if it means the default exports are left clunky. |
|
Opened #2389, which is much smaller. NodeNext users can switch to using the named export instead, side-stepping the default import/export issues. |
|
Closing in favor of #2389 |
What issue does this pull request resolve?
Closes #2132
What changes did you make?
Changed the way types are exported to improve support for users
module: node16/nodenextusers.Is there anything that requires more attention while reviewing?