-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: save import map URL to manifest #235
Conversation
@@ -10,4 +14,20 @@ const url = new URL(import.meta.url) | |||
const dirname = fileURLToPath(url) | |||
const fixturesDir = resolve(dirname, '..', 'fixtures') | |||
|
|||
export { fixturesDir, testLogger } | |||
const useFixture = async (fixtureName: string) => { |
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.
I created this helper and refactored all tests in bundler.test.ts
to use it because I realised our tests were not mimicking the structure observed in production. In production, the base path (e.g. /project
) is a parent to both the source directory (/project/netlify/edge-functions
) and the dist directory (/project/.netlify/edge-functions-dist
), but in tests we were using as dist directory a temporary path that is something like /var/tmp
.
With this helper, we copy the source directory into that temporary directory and make the base path.
} | ||
|
||
add(file: ImportMapFile) { | ||
this.files.push(file) | ||
} | ||
|
||
getContents() { | ||
let imports: Record<string, URL | null> = {} | ||
getContents(rootPath?: string) { |
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.
Shouldn't rootPath be mandatory? For example .toDataURL()
would now create a different import map?
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.
We can't have relative paths when the import map is described as a data URL, so in that situation we'll leave the paths as absolute.
* feat: save import map URL to manifest * fix: use POSIX paths * chore: fix integration test * chore: fix test * chore: remove unused file * chore: fix path in test Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Which problem is this pull request solving?
Import maps aren't working as expected at the moment for a couple of reasons.
First of all, we're not setting the
import_map_url
property in the isolate config, because we're not even writing that to the manifest.Also, we're writing absolute paths (from the host machine in the build system) into the import map, instead of converting those to ESZIP paths using the virtual root prefix.
This PR addresses that.
List other issues or pull requests related to this problem
Closes https://github.com/netlify/pod-compute/issues/313.