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

Load Web Worker script from a correct location #354

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

pjanik
Copy link
Member

@pjanik pjanik commented Feb 12, 2024

https://www.pivotaltracker.com/story/show/187028887
https://www.pivotaltracker.com/story/show/186957289

Another PR has been created to improve the new release process. The Web Worker script was being loaded from a local directory instead of the /version/v2.7.0 directory after the production release and that was causing the issue reported by Trudi.

I've explored better approaches, and there are two separate ones:

  • The recommended approach by Webpack 5, as detailed at https://webpack.js.org/guides/web-workers/, won't work because the project does not use ESM.
  • The worker-loader might work, but I was unable to pass custom URL parameters to the worker script to set up the configuration. Using postMessage is not feasible, as a significant amount of code requires certain values to be set at initialization.

So, I have opted to pass the deployment path to the TS script and use it there.

Copy link
Member

@dougmartin dougmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@433eros 433eros merged commit 41631bb into master Feb 12, 2024
10 checks passed
@433eros 433eros deleted the 187028887-fix-webworker branch February 12, 2024 22:03
Comment on lines +179 to +183
})] : []),
// Define DEPLOY_PATH globally so it can be used to load Web Worker script from a correct location
new webpack.DefinePlugin({
'__WEBPACK_DEPLOY_PATH__': JSON.stringify(DEPLOY_PATH || ".")
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjanik rather than doing this, perhaps you can use the built in webpack variable __webpack_public_path__? This is described here: https://github.com/concord-consortium/starter-projects/blob/master/doc/deploy.md

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @scytacki, I've made this change in #355.

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.

4 participants