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

Nested runAsync calls throw ReferenceError: Property 'global' doesn't exist #186

Open
mrousavy opened this issue May 3, 2024 · 3 comments
Assignees

Comments

@mrousavy
Copy link
Member

mrousavy commented May 3, 2024

When using nested runAsync calls like so:

const context = Worklets.defaultContext
context.runAsync(() => {
  'worklet'
  console.log('first!')
  Worklets.runOnJS(() => {
    'worklet'
    console.log('second!')

    try {
      const worklet = () => {
        'worklet'
        console.log('third!')
      }
      context.runAsync(worklet)
    } catch (e) {
      console.error(e)
    }
  })
})

The worklet creation throws an error:

-  ERROR  [ReferenceError: Property 'global' doesn't exist]

For some reason I couldn't reproduce this in this repo, but I can reproduce this 100% / always in react-native-vision-camera.

Funny thing is; this doesn't throw in context.runAsync(worklet), but rather in the const worklet = () => { ... } function creation.

Maybe it is a bug in the babel plugin?

I think we need two PRs related to this issue btw;

  1. Fixing the bug in the babel plugin to make sure this code works (hopefully this also fixes the bug where runAsync sometimes didn't run in release in VisionCamera Frame Processors)
  2. Add a try / catch handler to properly display error messages in Worklets (otherwise errors just get swallowed)
@mrousavy
Copy link
Member Author

mrousavy commented May 3, 2024

Actually, shouldn't this ReferenceError be propagated upwards through runOnJS as a Promise rejection? Maybe there's also something going wrong here, I remember @hannojg noticing that a .catch(..) call on our PromiseWrapper didn't work as intended.

Maybe worth mentioning that an Error instance cannot be passed between contexts and has to be unwrapped/wrapped first, see throwErrorOnJS from VisionCamera.
Or we do it natively through jsi::JSError.

@tfcornerstone
Copy link

tfcornerstone commented May 14, 2024

There were complaints of it working on non releases. This seems to be the only code references that distinguishes the release on the plugin. Possibly the bug is composed around the compacted source map on release versions.

  return ["production", "release"].includes(process.env.BABEL_ENV);
}

function shouldGenerateSourceMap() {
  if (isRelease()) {
    return false;
  }

  if (process.env.REANIMATED_PLUGIN_TESTS === "jest") {
    // We want to detect this, so we can disable source maps (because they break
    // snapshot tests with jest).
    return false;
  }

  return true;
}
...
if (shouldGenerateSourceMap()) {
    // Clear contents array (should be empty anyways)
    inputMap.sourcesContent = [];
    // Include source contents in source map, because Flipper/iframe is not
    // allowed to read files from disk.
    for (const sourceFile of inputMap.sources) {
      inputMap.sourcesContent.push(
        fs.readFileSync(sourceFile).toString("utf-8")
      );
    }
  }

  const includeSourceMap = shouldGenerateSourceMap();

  const transformed = transformSync(code, {
    plugins: [prependClosureVariablesIfNecessary()],
    compact: !includeSourceMap,
    sourceMaps: includeSourceMap,
    inputSourceMap: inputMap,
    ast: false,
    babelrc: false,
    configFile: false,
    comments: false,
  });

  let sourceMap;
  if (includeSourceMap) {
    sourceMap = convertSourceMap.fromObject(transformed.map).toObject();
    // sourcesContent field contains a full source code of the file which contains the worklet
    // and is not needed by the source map interpreter in order to symbolicate a stack trace.
    // Therefore, we remove it to reduce the bandwith and avoid sending it potentially multiple times
    // in files that contain multiple worklets. Along with sourcesContent.
    delete sourceMap.sourcesContent;
  }

  return [transformed.code, JSON.stringify(sourceMap)];
}```

@tremblerz
Copy link

Might be related #205

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

No branches or pull requests

4 participants