-
Notifications
You must be signed in to change notification settings - Fork 88
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
[Package Importer] Specs #1952
Merged
Merged
[Package Importer] Specs #1952
Changes from 1 commit
Commits
Show all changes
65 commits
Select commit
Hold shift + click to select a range
ddd98e7
Stub out Package Importer tests, add changeEntryPoint to sandbox
jamesnw b489a76
More tests
jamesnw 20ebaab
Parent directories
jamesnw 51214a6
Add tests for all compilation methods
jamesnw db1db07
Add package export tests
jamesnw 40f30ae
Tests for exports
jamesnw f4748eb
Clean up assertions
jamesnw 68eabbb
More legacy compilation tests
jamesnw 447ac0e
Clean up false positives, return promise from sandbox test
jamesnw 25500cb
Promisify render tests to prevent sandbox dirs from being deleted bef…
jamesnw a024ff0
Nest Package Importer tests in describe
jamesnw fad1cc3
Wildcard tests
jamesnw f5e413c
Test legacy importer order
jamesnw 1ddd748
Flesh out stubbed tests
jamesnw cbf39fb
Merge branch 'sass:main' into feature.package-importer
jamesnw c98b9ba
Merge branch 'feature.package-importer' of github.com:oddbird/sass-sp…
jamesnw 3466dc6
Change throw expectation
jamesnw 5e04ad9
Add browser-only test for no file system
jamesnw 7b9743f
Run test only in browser
jamesnw 83dc0a7
Review responses
jamesnw ccd2a77
Add full wildcard path test
jamesnw b49db2c
Move require.main.filename inside sandbox dir
jamesnw f96ba8d
Factor out common testPackageImporter logic
jamesnw 88d5ae6
Fix testPackageImporter call, add scoped packages and relative import…
jamesnw 551b5b7
Clean up paths
jamesnw 3bbb918
Check that no match in pkg importer continues to next importer
jamesnw 4230a0a
Fix typo
jamesnw 137c8f9
Handle invalid URLs
jamesnw 5a6382e
Adjust tests for Symbol and embedded host
jamesnw 3d66845
Update fake importer test
jamesnw 4308648
Test changes to entry point url
jamesnw 75b02ad
Address review
jamesnw 5b38195
Test for non-sass file result
jamesnw 247a136
Test throw if invalid package.json
jamesnw 0cd68eb
Add test for exports with and without .
jamesnw 41f3416
Merge branch 'main' of https://github.com/sass/sass-spec into feature…
jamesnw fd4fac4
Merge branch 'main' into feature.package-importer
jgerigmeyer 31b757a
Move to Node Package Importer Class, add test for explicit entry point
jamesnw 70241b4
Merge branch 'feature.package-importer' of github.com:oddbird/sass-sp…
jamesnw 1a292f5
lint
jamesnw a31051f
Fix importer test
jamesnw ad97abb
Add tests for package name errors
jamesnw 654b451
Temporarily log full stack trace on error
jgerigmeyer 3fdd163
add comment
jgerigmeyer bee93ca
Update error messages
jamesnw 2b6a3c5
Update legacy tests for new pkgImporter syntax
jgerigmeyer a6cdbdf
whitespace
jgerigmeyer 46b38be
update copyright year
jgerigmeyer 5f76314
Update package name tests
jamesnw 27ee25b
remove unnecessary type defs
jgerigmeyer 7860a91
Merge branch 'main' of https://github.com/sass/sass-spec into feature…
jamesnw b5edacb
Merge branch 'main' of https://github.com/sass/sass-spec into feature…
jamesnw 3f435d7
Make chdir async in sandbox
jamesnw a01db2a
Update expected errors
jamesnw d20e8b4
No error with invalid node package names
jamesnw b18ff2c
Address review
jgerigmeyer 7a5e0ce
Update entry point to directory
jamesnw ce63d68
Make pkg:bar unique for troubleshooting
jamesnw 029ce09
Lint
jamesnw 628add2
Log test error
jamesnw c841cce
Return chdir to ensure errors don't leak, use file url toString for a…
jamesnw 45561f4
Remove skips, relog
jamesnw adf2f4a
Try pathname
jamesnw 2e5b770
Use fileURLToPath
jamesnw 3263bc3
Pass entrypoint correctly
jamesnw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nex3 I needed to make this change after merging Shared Resources into
node-embedded-host
, as all async tests were failing. I think this is correct now (the callback can be async, so we need to await it, so that thefinally
doesn't happen before the callback resolves).However, it seemed worth flagging with you because the async
dart-sass
tests did not fail, and thenode-embedded-host
tests did not fail until after merging in Shared Resources, and we weren't able to track down any cause for this difference.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be that functions with
FutureOr<T>
return type in dart is not really an async function in JS, but a sync function that may return a Promise or T. This allows the some synchronous code path to be dispatched synchronously, and only code inside the returned Promise is dispatched asynchronously, therefore may lead to different execution order than how a true asynchronous function is dispatched.