-
Notifications
You must be signed in to change notification settings - Fork 45
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
Cross-origin isolation support in Chrome #67
base: main
Are you sure you want to change the base?
Conversation
…n=anonymous to injected script tag
@@ -8,7 +8,32 @@ export default { | |||
format: 'cjs', | |||
}, | |||
plugins: [ | |||
serve({ contentBase: '', port: Math.round(Math.random() * 10000) + 40000 }), | |||
live(), | |||
serve({ |
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.
Do you have an idea of how to test this in CI?
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.
hi @thgh, do you mean testing plugin serve with that specific configuration?
I suppose we could set GH actions with node.js to capture the output of rollup.
Would you require this to approve this PR? I see that there is no CI setup in place yet.
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.
Had a peek at how node-livereload does the server testing.
https://github.com/frantic0/node-livereload/blob/main/test/index.test.coffee
It is a travis CI setup and does include server configurations
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.
No requirement, just looking for inspiration
I'm not using this plugin actively so we should start doing automated testing to remain confident when releasing. I hope to make it a requirement soon.
removed and installed livereload to latest
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.
@thgh, thanks for the review, just pushed the changes
@@ -8,7 +8,32 @@ export default { | |||
format: 'cjs', | |||
}, | |||
plugins: [ | |||
serve({ contentBase: '', port: Math.round(Math.random() * 10000) + 40000 }), | |||
live(), | |||
serve({ |
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.
hi @thgh, do you mean testing plugin serve with that specific configuration?
I suppose we could set GH actions with node.js to capture the output of rollup.
Would you require this to approve this PR? I see that there is no CI setup in place yet.
@frantic0 thanks for the PR. Could you rebase it and fix the merge conflicts? |
This PR depends on an open PR in livereload: napcs/node-livereload#155 To land this, we will need patch-package or a published fork of livereload |
👋 I have this PR for LiveReload that addresses CORS and CORP - it's a different one than you referenced here.napcs/node-livereload#164 Would this be adequate for what you need? |
Closes #66