Skip to content

feat(jsr): improve jsr support#1186

Merged
antongolub merged 14 commits intogoogle:mainfrom
easymikey:build-jsr-lib
Apr 8, 2025
Merged

feat(jsr): improve jsr support#1186
antongolub merged 14 commits intogoogle:mainfrom
easymikey:build-jsr-lib

Conversation

@easymikey
Copy link
Contributor

@easymikey easymikey commented Apr 7, 2025

Fixes #1180

  • Add NodeJS specification types
  • Add supports for globalThis for Deno
  • Supports VERSION from jsr.io
  • Supports verdor-core with default exports
  • Add supports for cli.ts for jsr.io
  • Update size-limit
import { $, VERSION } from './src/index.ts'
import { main } from './src/cli.ts'

;(async () => {
  await $({ verbose: true, stdio: 'pipe' })`echo hello`

  const p = $`echo foo`
  console.log(p.stage)

  main()
})()

@antongolub

@easymikey easymikey changed the title feat(jsr): improve jsr supports feat(jsr): improve jsr support Apr 7, 2025
@antongolub
Copy link
Collaborator

antongolub commented Apr 8, 2025

@easymikey,

Apply rm -rf build to fix the bundle size issue.

@antongolub
Copy link
Collaborator

Let's make a bit shorter ver resolver:

export const VERSION: string = (() => {
  const f = (file: string) =>
    fs.readJsonSync(new URL(file, import.meta.url), { throws: false })
  return (f('../package.json') || f('../jsr.json'))?.version
})()

.nycrc Outdated
"reporter": ["html", "text"],
"lines": 98,
"branches": "90",
"branches": "89",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix this another way? Add some extra tests maybe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix this another way? Add some extra tests maybe.

I researched and found how to fix the branching like as globalThis.Deno ? globalThis.require('./cli.cjs') : module, so I lowered the coverage threshold. May be use istanbul ignore next directive, but I don't find anything in this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a closer look at the report

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

.size-limit.json Outdated
"name": "js parts",
"path": "build/*.{js,cjs}",
"limit": "815.7 kB",
"limit": "828.90 kB",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

817.1 kB ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 10more by accident, I will refund

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

return new Proxy<T>(api, {
get(_, key) {
return store.get(name)[key]
return store.get(name)[key] || store.get(name)?.default?.[key]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, I'd suggest a minor improvement:

const store = new Map<string, any>()
const override = store.set.bind(store)
const trydef = (v: any) => v?.default || v
const wrap = <T extends object>(name: string, api: T): T => {
  override(name, api)
  return new Proxy<T>(api, {
    get(_, key) {
      return trydef(store.get(name))?.[key]
    },
    apply(_, self, args) {
      return trydef(store.get(name) as TCallable).apply(self, args)
    },
  })
}

Copy link
Collaborator

@antongolub antongolub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the improvement!

@antongolub antongolub merged commit 698b53b into google:main Apr 8, 2025
26 checks passed
@antongolub
Copy link
Collaborator

antongolub commented Apr 8, 2025

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.

bug: jsr requires globalThis and smth is wrong with internal fs.readJsonSync type

2 participants