Ncc: Failing tests with ncc 0.20.1

[styfle]

The now-builders repo has 4 failing test fixtures and most of them appear to be regressions in ncc.

Update: Tracking issues -

• [x] https://github.com/zeit/ncc/issues/435
• [x] https://github.com/zeit/ncc/issues/440

Steps to reproduce

Please clone the repo and install dependencies:

git clone https://github.com/zeit/now-builders
cd now-builders
yarn
yarn bootstrap
yarn build

I went through each fixture and some fail differently with the CLI vs API.

• CLI: this means I ran the commands you see below
• API: this means programatic usage from @now/node index.ts

08-assets

cd packages/now-node/test/fixtures/08-assets
echo 'require("http").createServer(module.exports).listen(3000)' >> index.js 
npx @zeit/[email protected] build index.js --source-map
node dist

• Works from CLI
• Fails from API. Probably incorrect assets paths.

09-include-files

cd packages/now-node/test/fixtures/09-include-files
npm install
echo 'require("http").createServer(module.exports).listen(3000)' >> root.js 
npx @zeit/[email protected] build root.js --source-map
node dist

• Fails from CLI with "RuntimeException: E_MISSING_VIEW: Cannot render root.edge. Make sure the file exists at"
• Fails from API. This could probably be resolved by glob instead of emitting assets so hold off on changing this one for now.

14-stack-trace-ts

cd packages/now-node/test/fixtures/14-stack-trace-ts
echo 'require("http").createServer(handler).listen(3000)' >> index.ts 
npx @zeit/[email protected] build index.ts --source-map
node dist

• Works from CLI
• Fails from API with "callback was already called" 🤔

15-helpers

cd packages/now-node/test/fixtures/15-helpers/ts
echo 'require("http").createServer(listener).listen(3000)' >> index.ts 
npx @zeit/[email protected] build index.ts --source-map
node dist

• Fails from CLI with "TypeError: res.status is not a function"
• Fails from API with "callback was already called" 🤔

I think the TypeError can be ignored and probably fixing test 14 will fix 15.

[guybedford]

08-assets

The debugging information on this test shows:

Error: Fetched page https://test-g1rtmdzcg.now.sh/ does not contain asset1:190dd767190dd767190dd7,asset2:190dd767190dd767190dd7. Instead it contains An error occurred with this application.

How can we get the stack trace for whatever is happening above? I wasn't able to work out how to see this at all from the test case, so could not isolate this further.

09-include-files

This could probably be resolved by glob instead of emitting assets so hold off on changing this one for now.

I'm not sure I follow this?

I looked into the issue further and it really does come down to: _do we want any __dirname reference on its own to possibly emit an entire project directory?_.

The reason this becomes a problem is that we have other analysis expressions which can also resolve to dirname, eg path.resolve('.'), process.cwd() - would we want any instances of those expressions to also result in entire project directories being emitted? This is what happens in the ncc build itself in a stray process.cwd() expression resulting in the entire ncc folder being emitted.

I would strongly advise changing the test and not implementing this behaviour, but if it is absolutely necessary we can discuss very specific filtering techniques further. We need to carefully design how we want to define this space though.

14-stack-trace

Again, I have no idea how to isolate the test. If you can provide me a replication that I can hack on then I can look into this further. But since I don't know how to debug the test, I cannot determine the issue.

15-helpers

Agreed this sounds like a similar case.

[guybedford]

I managed to trace 14 and 15 down to https://github.com/zeit/ncc/issues/435.

[guybedford]

For 08 - if I navigate to the application directly, I can see the server outputting exactly asset1:3cbc3a523cbc3a523cbc3a,asset2:3cbc3a523cbc3a523cbc3a which is correct. Perhaps this is a test timing bug or something like that as opposed to an ncc bug.

[guybedford]

We finally traced down 08 to the following filtering issue by building with debugLog: true:

Skipping asset emission of /tmp/74b55224/subdirectory1/asset.txt for /tmp/74b55224/index.js as it is outside the process directory /var/task

Created https://github.com/zeit/ncc/issues/440.

[guybedford]

All of the ncc issues here have been resolved in 0.20.2.

14 and 15 should be fixed on the upgrade by default. If not, we can reinvestigate.

The now-node issues to do with over-filtering can be caught by providing the debugLog option to ncc to see logs like the above.

The filtering fix for 08 is to set filterAssetBase: path.resolve('/pkg/path') so that it doesn't use the process.cwd() filter base by default and instead knows that these root-level paths are ok /tmp/sdf or something I believe the path was.

[guybedford]

Closing, but please let's reopen if any of these come up again.

[ywg-jean]

hello @guybedford

I have updated my PR against now-builders to upgrade to [email protected]
unfortunately the tests are still failing: https://circleci.com/gh/zeit/now-builders/2662

08-assets

You comment suggests that the tests for assets (8 in both now-node and now-server) need to configure filterAssetBase to work properly.
Being unsure what the value for the option should be. I have tried both of the values that looked like path in the scope where ncc (entrypointPath and workPath) is called but neither resulted in a green test and I am way out of my depth there. revisiting everything I think I should pass path.resolve('workpath') but I get [Error: Rate limit exceeded] when I try to run the tests now :(
edit : after waiting for the limits to reset, path.resolve('workpath') fixed test 08.

09-include-files

In my understanding upgrading to [email protected] won't help.
I will try to rephrase the issue as I understand it, ncc doesn't know that it should package the templates directory.
Between 0.18.x and 0.20.x ncc started filtering paths more aggressively, and dynamically constructed paths can now be filtered out.
you advised changing the test but I am unsure I understand how it should be changed ... should it use path.join(__dirname ,...) as documented on https://edge.adonisjs.com/ and used in 08-assets or should it provide a different way to resolve the proper template path ?
edit : after waiting for the limits to reset using path.join(__dirname,'./templates') doesn't seem to be enough to fix this test.

14-stack-trace-ts

fails to build with the following error :

Cannot find name 'Error'.     

full trace :

ncc: Version 0.20.2
--
BUILD | Jun 26 2019 11:53:22:878 | /index.ts | ncc: Compiling file index.js
BUILD | Jun 26 2019 11:53:23:362 | /index.ts | ncc: Using [email protected] (ncc built-in)
BUILD | Jun 26 2019 11:53:23:516 | /index.ts | Error: [tsl] ERROR       TS6053: File '/tmp/1b3d251c/.build-utils/.builder/node_modules/@zeit/ncc/dist/ncc/loaders/typescript/lib/lib.esnext.d.ts' not found. 
[tsl] ERROR       TS2318: Cannot find global type 'Array'. 
[tsl] ERROR       TS2318: Cannot find global type 'Boolean'. 
[tsl] ERROR       TS2318: Cannot find global type 'CallableFunction'. 
[tsl] ERROR       TS2318: Cannot find global type 'Function'. 
[tsl] ERROR       TS2318: Cannot find global type 'IArguments'. 
[tsl] ERROR       TS2318: Cannot find global type 'NewableFunction'. 
[tsl] ERROR       TS2318: Cannot find global type 'Number'. 
[tsl] ERROR       TS2318: Cannot find global type 'Object'. 
[tsl] ERROR       TS2318: Cannot find global type 'RegExp'. 
[tsl] ERROR       TS2318: Cannot find global type 'String'. 
[tsl] ERROR in /tmp/1e2621cc/index.ts(4,17)       TS2304: Cannot find name 'Error'.     at compiler.close.n (evalmachine.<anonymous>:1:1539399)     at _promise0.then._result0 (eval at create (evalmachine.<anonymous>:1:273358), <anonymous>:13:1)     at <anonymous>

15-helpers

fails with

ncc: Using [email protected] (ncc built-in)
--
BUILD | Jun 26 2019 11:53:41:862 | /ts/index.ts | Error: [tsl] ERROR       TS6053: File '/tmp/2c9129f8/.build-utils/.builder/node_modules/@zeit/ncc/dist/ncc/loaders/typescript/lib/lib.esnext.d.ts' not found. 
[tsl] ERROR       TS2318: Cannot find global type 'Array'. 
[tsl] ERROR       TS2318: Cannot find global type 'Boolean'. 
[tsl] ERROR       TS2318: Cannot find global type 'CallableFunction'.
[tsl] ERROR       TS2318: Cannot find global type 'Function'. 
[tsl] ERROR       TS2318: Cannot find global type 'IArguments'. 
[tsl] ERROR       TS2318: Cannot find global type 'NewableFunction'. 
[tsl] ERROR       TS2318: Cannot find global type 'Number'. 
[tsl] ERROR       TS2318: Cannot find global type 'Object'. 
[tsl] ERROR       TS2318: Cannot find global type 'RegExp'. 
[tsl] ERROR       TS2318: Cannot find global type 'String'.     at compiler.close.n (evalmachine.<anonymous>:1:1539399)     at _promise0.then._result0 (eval at create (evalmachine.<anonymous>:1:273358), <anonymous>:13:1)     at <anonymous>

[styfle]

@guybedford I confirmed some of these are still bugs.

• 08-assets: Works great, passes tests!
• 09-include-files: This is still a mystery. This one actually isn't testing ncc. It's testing the config.includeFiles option to make sure users can add files even if ncc can't find them. Perhaps this should glob relative to the rootPath. Again, doesn't seem like an ncc issue but very strange that it shows up when bumping ncc version.
• 14-stack-trace-ts: Definitely still a bug with ncc and typescript
• 15-helpers: Definitely still a bug with ncc and typescript

[guybedford]

Thanks @styfle - 14,15 seems like another builtin TypeScript bug and I
believe I know the issue. Will aim to post a patch in the next few days.
The workaround can be to copy the dist/ncc/loaders/ts-loader.js file in the
ncc 18 build over the 0.20 build and publish that as this file is the same
bit just an internal build issue.

On Wed, 26 Jun 2019 at 18:44, Steven [e-mail] wrote:

@guybedford https://github.com/guybedford I confirmed some of these are
still bugs.

• 08-assets: Works great, passes tests!
• 09-include-files: This is still a mystery. This one actually isn't
  testing ncc. It's testing the config.includeFiles option to make sure
  users can add files even if ncc can't find them. Perhaps this should glob
  relative to the rootPath. Again, doesn't seem like an ncc issue but very
  strange that it shows up when bumping ncc verison.
• 14-stack-trace-ts: Definitely still a bug with ncc and typescript
• 15-helpers: Definitely still a bug with ncc and typescript

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/zeit/ncc/issues/434?email_source=notifications&email_token=AAESFSR4Q6XPXU3Q4UTXOLLP4OMG7A5CNFSM4HZNVPWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYUEE3Q#issuecomment-505954926,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAESFSSANZY3MMQRX7NC67DP4OMG7ANCNFSM4HZNVPWA
.

[styfle]

@guybedford If I understand correctly, you are suggesting to get a copy of ts-loader from 0.18.5 and overwrite the existing ts-loader file in 0.20.2?

What about ts-loader.js.cache and ts-loader.js.cache.js? Do those need to be copied as well?

[ywg-jean]

@styfle I guess we were looking into this at the same time, I was writing :

@guybedford
I tried copying the ts-loader.js from a packaged 0.18.5 ncc to src/loaders/ts-loader.js
and ran the tests : it more or less exploded.
I then tried copying the src/loaders/ts-loader.js from https://github.com/zeit/ncc/blob/0.18.5/src/loaders/ts-loader.js and am running the tests right now (they take forever :( )
The last option I can think of is that you suggested to copy the ts-loader.js from a packaged 0.18.5 ncc into a packaged 0.20.2 ncc. We might be able to repackage that but is it really desirable to publish this as an official ncc version (for which the source code would basically be missing)

[guybedford]

Yes, it's ts-loader.js.cache.

[styfle]

@guybedford I tried every combination of copying ts-loader, ts-loader.js.cache, and ts-loader.js.cache.js.

It always results in the same error:

ncc: Version 0.20.2
ncc: Compiling file index.js
ncc: Using [email protected] (ncc built-in)
Error: [tsl] ERROR TS6053: File '/tmp/619ce465/.build-utils/.builder/node_modules/@zeit/ncc/dist/ncc/loaders/typescript/lib/lib.esnext.d.ts' not found.
[tsl] ERROR
      TS2318: Cannot find global type 'Array'.
[tsl] ERROR
      TS2318: Cannot find global type 'Boolean'.
[tsl] ERROR
      TS2318: Cannot find global type 'CallableFunction'.
[tsl] ERROR
      TS2318: Cannot find global type 'Function'.
[tsl] ERROR
      TS2318: Cannot find global type 'IArguments'.
[tsl] ERROR
      TS2318: Cannot find global type 'NewableFunction'.
[tsl] ERROR
      TS2318: Cannot find global type 'Number'.
[tsl] ERROR
      TS2318: Cannot find global type 'Object'.
[tsl] ERROR
      TS2318: Cannot find global type 'RegExp'.
[tsl] ERROR
      TS2318: Cannot find global type 'String'.
    at compiler.close.n (evalmachine.<anonymous>:1:1539399)
    at _promise0.then._result0 (eval at create (evalmachine.<anonymous>:1:273358), <anonymous>:13:1)
    at <anonymous>

[guybedford]

Weird, as that should be identical to the 0.18 behaviour... sure I will take a look first thing Monday.

[guybedford]

These are all fixed now in the latest ncc. Will continue to track the TypeScript workaround in https://github.com/zeit/ncc/issues/435.