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

Allow threads.js to function even with webpack node externals #224

Merged
merged 2 commits into from
Mar 31, 2020
Merged

Allow threads.js to function even with webpack node externals #224

merged 2 commits into from
Mar 31, 2020

Conversation

chingyawhao
Copy link
Contributor

@chingyawhao chingyawhao commented Mar 24, 2020

Solving #222

@chingyawhao
Copy link
Contributor Author

@andywer this would handle the absolute path returned by rebaseScriptPath() that you mentioned right?
I am assuming the absolute path you are talking about is something like /Users/chingyawhao/Documents

@andywer
Copy link
Owner

andywer commented Mar 25, 2020

Cool! Pushed a commit introducing some minor code improvements. Yes, that was the absolute path that I meant – only replaced the check with path.isAbsolute(), so we can be sure it works with Windows paths, too.

That's some great stuff right here 🙂

@chingyawhao
Copy link
Contributor Author

Thanks @andywer

I have no idea how to do testing for this scenario though, I tried adding webpack-node-externals to the existing node test before these changes but it did not fail

@chingyawhao chingyawhao reopened this Mar 27, 2020
@andywer
Copy link
Owner

andywer commented Mar 31, 2020

@chingyawhao Yeah, I was thinking the same. Unfortunately I don't really have time to look into this right now either…

We should probably just merge the fix. Do you need it urgently, btw?

@chingyawhao
Copy link
Contributor Author

I'm able to make the changes inside node_modules so its no rush

@andywer andywer merged commit cb3cbf9 into andywer:master Mar 31, 2020
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