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

Workspaces #64

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Workspaces #64

wants to merge 1 commit into from

Conversation

lishaduck
Copy link
Contributor

@lishaduck lishaduck commented Jun 7, 2024

Deno is rolling out a new feature called workspaces.
It enables JSR monorepos to link to packages locally.
While libs is far less interconnected than deno_std, I figured that it might help if @libs/testing got some extra "tests" by having all the packages link to the local copy in development.

It should also enable a single dependency resolve and (probably) obsolete deno_config.ts, as I believe it automatically syncs formatting/linting/etc.

Copy link
Owner

@lowlighter lowlighter left a comment

Choose a reason for hiding this comment

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

The workspaces setting seems blocked by vercel-community/deno

Everything in .vscode/.github looks good to me so if you'd like to merge these changes now you could open a separate PR

As for the lint.tags I'd prefer keeping it less verbose since it's already the default

bundle/deno.jsonc Outdated Show resolved Hide resolved
"run/",
"testing/",
"typing/",
"xml/"
Copy link
Owner

Choose a reason for hiding this comment

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

It makes vercel deployments fail with the following:

error: Workspace member 'bundle/' has no deno.json file ('/vercel/path1/bundle/deno.json')
Error: Build script failed with exit code 1

I think it's related to denoland/deno#23343 and the fact that the vercel-community/deno uses 1.42.1 currently.

I know you can pass a --version flag to them according to their docs, but I'll prefer it to not do as it does not make sense to break lambdas just because of a conf change which is enterily not related to actual code.

I'll wait for a version bump on their side before merging this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deno workspaces enable JSR dependencies to resolve to local copies.
This essentially turns all of the packages' tests into tests for `@libs/testing`.
Copy link
Contributor Author

@lishaduck lishaduck left a comment

Choose a reason for hiding this comment

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

As for the lint.tags I'd prefer keeping it less verbose since it's already the default

Yeah, I hadn't realized it would make such a big diff when I made the change originally. It's reverted now.

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.

None yet

2 participants