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

feat!: implement smart wrapping for all contexts #140

Closed
wants to merge 8 commits into from

Conversation

shadowspawn
Copy link
Member

@shadowspawn shadowspawn commented May 13, 2023

Problem

Solution

  • add cjs based smart wrapping for import conditional export
  • add esm based smart wrapping for deno
  • tweak types for deno and cjs [sic] to make opts optional
  • tweak types for deno and cjs [sic] to make opts.width optional
  • add default width for deno
  • add "browser" entry point for esm-only clients, including browsers (i.e. no use of CommonJS). This follows pattern used in yargs.

This PR takes a different approach to #139, which used import map to have two copies of dependencies.

This changes the main esm entry point to contain hybrid ESM/CommonJS imports. This won't affect usage from node but may affect some clients. This is probably a breaking change which needs to go in a semver major release?

To do

  • update tests
  • browser?!

Out of scope

  • properly export type definitions, cjs and esm and deno and browser

- default width for deno
- make opts optional for clients
- make opts.width optional for clients
@shadowspawn
Copy link
Member Author

I am a command-line jockey not a web guru. Is there an easy way of testing the hypothetical "browser" endpoint? I have written one but not having much luck testing it! (I'll push it anyway so visible.)

@shadowspawn
Copy link
Member Author

shadowspawn commented May 14, 2023

I hacked around with the browser code importing URLs on jsfiddle.net and functional using either of esm.sh or cdn.skypack.dev, which are mentioned in the deno CDN notes:

https://deno.land/manual@v1.28.3/node/cdns

@shadowspawn shadowspawn changed the title Implement smart wrapping for all contexts feature!: implement smart wrapping for all contexts May 14, 2023
@shadowspawn shadowspawn changed the title feature!: implement smart wrapping for all contexts feat!: implement smart wrapping for all contexts May 14, 2023
@shadowspawn shadowspawn marked this pull request as ready for review May 14, 2023 05:36
@shadowspawn
Copy link
Member Author

PR #119 has a rollup configuration that works with the ESM variations, so moving forward rather than using the cjs like I did here. Moved back to draft while follow-up with that.

@shadowspawn
Copy link
Member Author

I am closing this in favour of #143. I'll spin off separate PRs for the typings improvements and Deno terminal width.

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.

1 participant