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

Switch to pnpm #670

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft

Switch to pnpm #670

wants to merge 22 commits into from

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jan 5, 2024

yarn@v1 can't be trusted. this has been known for ages.
Also, newer tooling isn't supporting yarn@v1.

unblocks: #668

Changes:

  • remove yarn
  • add pnpm
  • Support isolated packages
    (a requirement to get types passing)
    Previously, everything was all loosey goosey / incorrect.
    • @glimmer/runtime was not specified, adding it adds the ESM version of Glimmer. No modern version of @glimmer/* exports CJS, and thus is not compatible with Glint (which is compiled to CJS (because VSCode extensions don't support ESM))
    • @glint/core was added to any package that invokes the glint bin. This is required, because bin directories are are local to each package.
    • Other glint packages were added to each package that imports from them, because there is no more hoisting, which means TS can't find the packages unless they are declared in the package.json. This has been done as devDependencies
    • We can't have dependencies loosely importing from all over the place, so extra packages have been extracted for integration testing
      • Example: @glint/core cannot have tests that consume any environment package, because the environment packages depend on @glint/core
    • There are resolution issues with the environments. :(

@NullVoxPopuli NullVoxPopuli added the internal Changes that don't impact the published packages label Jan 5, 2024
@@ -1,9 +1,8 @@
import { SafeString } from '@glimmer/runtime';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can't import from Glimmer, because Glimmer is ESM, and Glint is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, the tests could be extracted to an ESM package, but then we'd lose the ability to test private things.

A solution could be vitest's in-source testing.
This would require revamping how all the glint packages are built though. (not using tsc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes that don't impact the published packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant