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

Fix port data source import on Windows #282

Conversation

j-maas
Copy link
Contributor

@j-maas j-maas commented Mar 20, 2022

When trying to use a port-data-source.js on Windows, I run into another path-related issue:

Trace: DataSource.Port.send "imageSources" was called, but I couldn't find a port definitions file. Create a 'port-data-source.ts' or 'port-data-source.js' file and export a imageSources function.
    at C:\Users\Johannes\Code\elm-pages-3-alpha-starter\elm-pages\generator\src\request-cache.js:84:19

The cause is that we import() the file using an absolute path, but Windows-style absolute paths do not work and we have to use file:// URLs instead.

@j-maas
Copy link
Contributor Author

j-maas commented Mar 20, 2022

This is trickier than I thought. If I provoke a syntax error, the file will not be compiled. But the dev server will actually start. So when I later call a data source port, I get an error saying it couldn't find the file which is confusing: It means it can't find the compiled file, since compilation failed, but the data-source-port.js actually exists.

I've tried a few things, but I'm getting tired and will leave this as is for now. I think it might already be an improvement and we could weed out this case separately. But you decide what's the best option for you. ;)

@j-maas j-maas marked this pull request as ready for review March 20, 2022 14:49
@dillonkearns
Copy link
Owner

Sounds good, thanks so much for the PR! That's especially helpful for windows fixes since I don't have an environment to reproduce and iterate on fixes, so I really appreciate it!

The distinction between syntax errors vs. missing port-data-source file I think is something I'll have to deal with regardless of platform, so I'll go ahead and merge this in and work on that separately (that was already on my todo list). Thank you!

@dillonkearns dillonkearns merged commit 3790fbe into dillonkearns:serverless-latest Mar 25, 2022
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.

2 participants