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

Import rework #59

Merged
merged 23 commits into from
Feb 4, 2025
Merged

Import rework #59

merged 23 commits into from
Feb 4, 2025

Conversation

stefnotch
Copy link
Contributor

@stefnotch stefnotch commented Jan 27, 2025

Reworks the imports grammar and types

@stefnotch stefnotch marked this pull request as ready for review February 1, 2025 19:46
Copy link
Contributor

@mighdoll mighdoll left a comment

Choose a reason for hiding this comment

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

wallaby and vitest are failing when I try it (after pnpm i). Does it work for you?
I get:
Tests 3 failed | 330 passed | 4 skipped (337)

@mighdoll
Copy link
Contributor

mighdoll commented Feb 2, 2025

awesome to see the import cases all passing

@mighdoll
Copy link
Contributor

mighdoll commented Feb 2, 2025

I'm surprised there aren't new import cases despite some spec changes, etc. Don't hold those corner cases in! add some cases while imports are relatively fresh in your mind? we'll use 'em a lot and for other tools too. (obviously can be separate commit)

Copy link
Contributor

@mighdoll mighdoll left a comment

Choose a reason for hiding this comment

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

tests need to pass of course. Otherwise fix or defer minor comments and lgtm.

@stefnotch
Copy link
Contributor Author

stefnotch commented Feb 3, 2025

wallaby and vitest are failing when I try it (after pnpm i). Does it work for you? I get: Tests 3 failed | 330 passed | 4 skipped (337)

Apparently running vitest from the root is still broken for me

D:\Coding\GitHub\Tools\wesl-js\linker> pnpm run test --ui                                                                                                                                                                                                                                   02/03/2025 02:22:50 PM

> root@ test D:\Coding\GitHub\Tools\wesl-js\linker
> vitest "--ui"

failed to load config from D:\Coding\GitHub\Tools\wesl-js\linker\packages\plugin-test\vite.config.ts

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Startup Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error [ERR_MODULE_NOT_FOUND]: Cannot find module 'D:\Coding\GitHub\Tools\wesl-js\linker\packages\plugin\node_modules\wesl\dist\index.js' imported from D:\Coding\GitHub\Tools\wesl-js\linker\packages\plugin\dist\chunk-2PLORNUK.js
    at finalizeResolution (node:internal/modules/esm/resolve:264:11)
    at moduleResolve (node:internal/modules/esm/resolve:917:10)
    at defaultResolve (node:internal/modules/esm/resolve:1130:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:396:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:365:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:240:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:85:39)
    at link (node:internal/modules/esm/module_job:84:36) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file:///D:/Coding/GitHub/Tools/wesl-js/linker/packages/plugin/node_modules/wesl/dist/index.js'
}

Which tests fail for you? For me, mini-parse and linker have all their tests pass.

Aah, bulk-test has some tests that time out!

@stefnotch
Copy link
Contributor Author

This is ready to merge. While the spec has changed, it hasn't changed in a significant enough way to actually impact this.

(I'll update the import parser to look 100% like the spec in a future PR)

@stefnotch
Copy link
Contributor Author

I'm merging this

@stefnotch stefnotch merged commit 08e3971 into main Feb 4, 2025
@stefnotch stefnotch mentioned this pull request Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants