Error on Custom Plugin

[luii]

I've created a decompress plugin for start and get this error:

yarn start testsomething
yarn run v1.5.1
$ node packages/cli/src/index.js testsomething
/home/luii/Dokumente/git/start/packages/reporter-verbose/src/index.ts:1
ReferenceError: _70d‍ is not defined
    at Object.<anonymous> (/home/luii/Dokumente/git/start/packages/reporter-verbose/src/index.ts:1)
    at Module._compile (/home/luii/Dokumente/git/start/node_modules/pirates/lib/index.js:91:24)
error An unexpected error occurred: "Command failed.
Exit code: 1
Command: sh
Arguments: -c node packages/cli/src/index.js testsomething
Directory: /home/luii/Dokumente/git/start
Output:
".
info If you think this is a bug, please open a bug report with the information provided in "/home/luii/Dokumente/git/start/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

This is my plugin:

import plugin from '@start/plugin/src/'
import { DecompressOptions } from 'decompress';

export default (outDirRelative: string, options?: DecompressOptions) =>
  plugin('decompress', async ({ files, logFile }) => {
    const path = await import('path')
    const { default: decompress } = await import('decompress')
    const { default: movePath } = await import('move-path')
    
    return Promise.all(
      files.map(async file => {
        try {
          const outFile = movePath(file.path, outDirRelative)
          const outDir = path.dirname(outFile)

          logFile(outDir)
  
          await decompress(file.path, outDir, options)
          return await { path: outDir, data: null, map: null }
        } catch (error) {
          console.error(error)
        }
      })
    )
  })

And this is in the tasks/index.ts:

export const testsomething = () => 
  sequence(
    find('packages/plugin-decompress/test/fixtures/file.tar'),
    decompress('tasks/')
  )

[deepsweet]

Does rm -rf node_modules/.cache help?

@jdalton Could you please take a look? I believe it's related to ESM loader, most likely when it's wired together with Babel. Happens for me from time to time as well, especially when I upgrade the loader, but I never had a chance to have it reproducible and narrowed down. Wiping loader cache always helps.

[luii]

@deepsweet wow, just deleted it and it works like magic 👍
should i make a PR that you can publish it under "@start"?

[deepsweet]

@luii yes, please make a PR, I'll do the rest of minor tweaking 👍 thanks!

[luii]

I would love to!

[jdalton]

From the run without the plugin to the next run with the plugin was there any other error?
Is @babel/register's cache option changed to anything other than the default node_modules/.cache/@babel/register? Are there any other caches at play?

@luii Can you reproduce this error consistently? For example run it without the plugin. Next add the plugin and run it again (does it error?). If it does could you create a small repro repo with the repro case?

[luii]

@jdalton No unfortunatly not. I didnt changed anything, also cloned it fresh and retried the steps i've made. If it happens again i let you know as soon as possible!

[jdalton]

@deepsweet The cache miss on esm upgrades makes sense. When esm is upgraded it invalidates its old cache (and creates new entries). The babel/register cache still holds on to the old rutnime entries and so it'll result in that runtime error. The problem is babel/register's cache is too aggressive (caching on startup and shutdown). So if I detect an upgrade I can begin to clear their cache but it'll take 2 runs to complete. This can happen more naturally during a syntax error. For example the syntax error is thrown, I delete our esm cache and mark Babel's as dirty (as the process exits). On startup (before Babel runs) I detect the dirty Babel cache and clear it. The problem is on an upgrade there is no natural error case so the detection and clearing is a little more clunky. I could use post-install hooks for the npm package but those don't work in yarn.

In cases of an upgrade I can detect that a previous esm version is used and that babel/register is used and simply exit early with a message that we performed cleanup and to rerun the script... what do you think @deepsweet?

Update:

I guess I could detect the version change on-startup before Babel runs and just delete their cache without 2 passes.

[deepsweet]

@jdalton no other caches are involved, the only custom thing passed to @babel/register is "extensions": [".ts", ".js"] to handle TypeScript files.

I have just reproduced a similar issue:

rm -rf node_modules/
yarn
yarn start packs plugin-sequence plugin-lib-babel
yarn start packs plugin-sequence plugin-lib-babel
yarn start packs plugin-sequence plugin-lib-babel
yarn test

test.tape: _a54‍ is not defined

ReferenceError: _a54‍ is not defined
    Object.<anonymous> (packages/reporter-verbose/src/index.ts:1)
    Module.replacementCompile (node_modules/append-transform/index.js:58:13)
    Module._compile (node_modules/pirates/lib/index.js:91:24)

I have an idea: because the issue comes up so randomly it must be related to some race. And I have a lot of racing things here: packs task runs pack task for each argument in a separate child process + inside of each pack task there is a parallelism again, it runs build + dts in child-child processes on its own.

I think the commands above are still not 100% reproducible, but my idea is to run things in parallel many times to catch that race condition. If I'm right maybe it can be solved by using an approach similar to graceful-fs for writing cache (busy files, retries, etc)?

[jdalton]

append-transform? Is there nyc code coverage going on too?

[deepsweet]

@jdalton I'm not 100% sure again, but as I recall the upgrade issue doesn't come up every time... if it's detectable from the loader side and you can clean up the cache manually then in my opinion it's totally fine to do so.

There is no nyc in the pipeline, and I'm not sure what append-transform is 😨

[deepsweet]

Ah, it's related to Istanbul which I'm using totally manually.

$ yarn why append-transform

info Reasons this module exists
   - "_project_#@start#plugin-lib-istanbul#istanbul-lib-hook" depends on it
   - Hoisted from "_project_#@start#plugin-lib-istanbul#istanbul-lib-hook#append-transform"
   - Hoisted from "_project_#@start#plugin-lib-istanbul#istanbul-api#istanbul-lib-hook#append-transform"

[deepsweet]

I think it's in the stack trace because this time it fails on "coverage" plugin (randomly?). The first thing is the same, packages/reporter-verbose/src/index.ts:1.

From time to time I can catch it even without running tests, but after some parallel races for sure.

[jdalton]

Cool!

Okay, I have a couple things to try out.

Update:

@deepsweet Where are you caching instanbul's output?

[deepsweet]

@jdalton I don't think Istanbul has its own cache, but it uses inline Source Maps which comes from Babel.

const sourceMapRaw = getSourceMapFromSource(source)
let sourceMapObject = null

if (sourceMapRaw) {
  sourceMapObject = sourceMapRaw.toObject()
}

const result = instrumenter.instrumentSync(source, file, sourceMapObject)

[deepsweet]

At the same time Istanbul itself is using Babel internally to traverse AST and instrument code.

[jdalton]

I have some logic to handle invalidating esm caches for nyc (nyc is the istanbul cli). Not sure how instrumentation works outside of that (in terms of temp folders it creates).

[deepsweet]

@jdalton the only related thing I've found https://github.com/istanbuljs/istanbuljs/blob/60fd29196fa71dead28431bf0a6dbfe916faa607/packages/istanbul-lib-source-maps/lib/source-store.js#L41

[jdalton]

Okay, I'm clearing the babel cache for more things now and invalidating esm and babel caches on upgrades. It may not solve everything but it's getting it into a better place.

If there's continued problems with a combo of istanbul and your start you might consider clearing caches as part of the istanbul run. The node_modules/.cache folder is used by ava, @babel/register, esm, nyc, and others.

Update:

esm v3.0.46 is released 🎉