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

dist/joi-browser.min.js doesn't work in workers #2251

Closed
jandppw opened this issue Dec 18, 2019 · 4 comments · Fixed by #2624
Closed

dist/joi-browser.min.js doesn't work in workers #2251

jandppw opened this issue Dec 18, 2019 · 4 comments · Fixed by #2624
Assignees
Labels
bug Bug or defect
Milestone

Comments

@jandppw
Copy link

jandppw commented Dec 18, 2019

Support plan

  • which support plan is this issue covered by? Community
  • is this issue currently blocking your project? no:
  • is this issue affecting a production system? no:

Context

  • node version: v12.13.0
  • module version with issue: 16.1.8
  • last module version without issue: N/A
  • environment (e.g. node, browser, native): browser, worker
  • used with (e.g. hapi application, another framework, standalone, ...): Angular
  • any other relevant information:

What are you trying to achieve or the steps to reproduce?

When using joi in a web application, e.g. with Angular, dist/joi-browser.min.js is spliced in the web application.

dist/joi-browser.min.js uses UMD. In the UMD preamble, there is a reference to window (used as fallback when no module loader is recognized). This fails when joi is used in a web worker, because in that context there is no global window.

What was the result you got?

Error: window is undefined

What result did you expect?

No error

Request

Is there a way to build dist/joi-browser.min.js without the reference to window?

Workaround

In a modern web application this path of the UMD preamble isn't used.

We created a quick fix, that replaces the reference to window with undefined in node_modules/@hapi/joi/dist/joi-browser.min.js, and use it in postinstall.

File ./scripts/quickfix-joi.js:

#!/usr/bin/env node

/**
 * `joi-browser.min.js` is build with UMD, which in this case refers to `window`.
 * That doesn't work in web workers (because there is no `window` global).
 *
 * However, the UMD path where the `window` reference is not used in our project.
 *
 * This quick fix replaces the reference to `window` in `joi-browser.min.js` with `undefined`.
 * It is to be executed in `postinstall`.
 */
const fs = require('fs').promises;

const joiBrowserPath = './node_modules/@hapi/joi/dist/joi-browser.min.js';
const options = { encoding: 'utf8' };

async function doIt() {
  const joiSrc = await fs.readFile(joiBrowserPath, options);
  // Replace the window object, which doesn't exist in a worker, with undefined
  // This parameter isn't used in our project anyway.
  const result = joiSrc.replace('window', 'undefined');
  await fs.writeFile(joiBrowserPath, result, options);
  console.log(`Doctored \`${joiBrowserPath}\` successfully.`);
}

console.log(
  `Doctoring \`${joiBrowserPath}\`.
It uses \`window\` in the UMD preliminary, but we don't use that. We replace it with \`undefined\`
…`
);
doIt();

In package.json:

{
  […]
  "scripts": {
    […]
    "postinstall": "node ./scripts/quickfix-joi.js"
  },
  […]
}
@jandppw jandppw added the support Questions, discussions, and general support label Dec 18, 2019
@hueniverse
Copy link
Contributor

@Marsup is this something we want to support/handle?

@Marsup
Copy link
Collaborator

Marsup commented Dec 23, 2019

There should be nothing preventing joi to work there, but the wrangler issue doesn't provide much help in how to achieve that.

@hueniverse
Copy link
Contributor

Unfortunately, no community resources were available to help resolve this issue after two weeks, and it is being closed. We close unresolved community issues to keep the issue tracker organized and effective.

@hueniverse hueniverse self-assigned this Jan 4, 2020
@hueniverse hueniverse added bug Bug or defect and removed support Questions, discussions, and general support labels Aug 1, 2021
@hueniverse hueniverse added this to the 17.4.2 milestone Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants