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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No errors for ESM module resolution, while errors are expected #158

Open
oxcafedead opened this issue Mar 5, 2024 · 3 comments
Open

No errors for ESM module resolution, while errors are expected #158

oxcafedead opened this issue Mar 5, 2024 · 3 comments

Comments

@oxcafedead
Copy link

The lib: https://www.npmjs.com/package/@microlabs/otel-cf-workers

The analysis: https://arethetypeswrong.github.io/?p=%40microlabs%2Fotel-cf-workers%401.0.0-rc.23

Shows all is ok, however, if we navigate to the lib, we'll clearly see incorrect import statement at https://github.com/evanderkoogh/otel-cf-workers/blob/v1.0.0-rc.23/src/instrumentation/env.ts#L6

I have also checked dist in the npm package, the same import.
It fails the build locally trying to use this lib in ESM project.

@andrewbranch
Copy link
Collaborator

Here’s that file in the built package: https://unpkg.com/browse/@microlabs/[email protected]/dist/esm/instrumentation/env.d.ts

It doesn’t have any imports.

@andrewbranch
Copy link
Collaborator

andrewbranch commented Mar 5, 2024

Ah, I see you must be looking at the JavaScript: https://unpkg.com/browse/@microlabs/[email protected]/dist/esm/instrumentation/env.js

attw is built under the assumption that most immediate runtime crashes would be caught by other means; the tricky thing is getting the types right when the JS build and .d.ts build are decoupled somehow. So, these JS files are not analyzed for broken imports. Usually what we see is a .js file that works and a .d.ts file that disagrees and implies the existence of a .js file that would be broken.

It’s not totally out of the question to expand this analysis, but I’m not sure it really aligns with the goals of the project. attw is never going to be a substitute for basic runtime validation.

@oxcafedead
Copy link
Author

I don't possess some deep knowledge in js/ts modules resolution, sorry if this is off-topic.
I've just read about mandatory extension for ESM resolution, but it relates for pure JS:
https://nodejs.org/api/esm.html#mandatory-file-extensions

So, apologies if this is not relevant for this lib. Please close this ticket if you don't see what can be done here further.

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

No branches or pull requests

2 participants