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

specify output dir in dist #163

Closed
wants to merge 5 commits into from
Closed

Conversation

pkarl
Copy link
Contributor

@pkarl pkarl commented Sep 5, 2024

updated tsconfig for esm + cjs to explicitly set output dirs; added package info embed to avoid package.json path resolution issue

Fixes #162

What this PR solves / how to test:

  • specify output dirs in tsconfig for esm + cjs
  • use build-time generated js file for name + version instead of package.json resolution

testing env:

  • node v20.16.0
  • typescript@5.4.4

to test:

  1. create a cloudflare worker via npm create cloudflare@latest -- my-first-worker
  2. cd my-first-worker
  3. confirm worker runs with npm run dev
  4. install latest otel cf workers with npm i @microlabs/otel-cf-workers
  5. run npm run dev, should see error per import / resolution error #162

@pkarl pkarl changed the title fix output dir in dist specify output dir in dist Sep 5, 2024
@parisholley
Copy link

just ran into this :) @evanderkoogh any chance of a merge?

@jahands
Copy link
Collaborator

jahands commented Sep 8, 2024

Why is the dist directory committed in this PR?

@pkarl
Copy link
Contributor Author

pkarl commented Sep 8, 2024 via email

@pkarl
Copy link
Contributor Author

pkarl commented Sep 9, 2024

looks like @evanderkoogh modded to solve, closing this PR

@pkarl pkarl closed this Sep 9, 2024
@evanderkoogh
Copy link
Owner

Lol.. I was just about to leave a comment here that I had found another way to fix it.. but thanks for your effort none the less!

@oldbettie
Copy link
Contributor

is this fixed? It fails I am not sure where to go from here

@pkarl
Copy link
Contributor Author

pkarl commented Sep 13, 2024

no it appears to still be borked.

@pkarl pkarl reopened this Sep 13, 2024
@pkarl
Copy link
Contributor Author

pkarl commented Sep 13, 2024

attn @evanderkoogh this still needs to be addressed. See the tsconfig changes in this PR. If you want I can re-PR with your versions changes and remove the whole dist mess from this

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

Successfully merging this pull request may close these issues.

import / resolution error
5 participants