fix: requestFragment option type

[armand1m]

Problem

Currently, when trying to create a requestFragment handler in typescript, the current type says that I should implement the fragment handler like this:

import { IncomingMessage } from "http";
import { Url } from "url";

import requestFragmentFn from "node-tailor/lib/request-fragment";

/** had to hardcode these from the index.d.ts since they're not exported */
interface IAttributes {
  id: string;
  src: string;
  async?: boolean;
  fallbackUrl?: string;
  primary?: boolean;
  public?: boolean;
  [key: string]: any;
}

const handleFragment = (
  /** the types are currently demanding that I should define this filterHeaders */
  filterHeaders: (attributes: IAttributes, req: IncomingMessage) => object,
  fragmentUrl: Url,
  fragmentAttributes: IAttributes,
  request: IncomingMessage,
  span = null,
) => {
  /* imagine some logic here */

  return requestFragmentFn(filterHeaders)(fragmentUrl, fragmentAttributes, request, span);
};

export default handleFragment;

import Tailor from "node-tailor";
import requestFragment from "./handlers/fragment";

const tailor = new Tailor({
  requestFragment,
});

which results in problems since the actual call in the source code is not actually like this

This PR fixes it by making it respect the actual implementation usage.

[vigneshshanmugam]

Thanks @armand1m. Good catch.

[vigneshshanmugam]

👍

[armand1m]

I'll also have to open one for the fetchTemplate soon as the types are also misleading on that one

[vigneshshanmugam]

@armand1m Sure, Please do

[vigneshshanmugam]

👍

[oporkka]

Is this PR waiting for something to be added?

[armand1m]

Is this PR waiting for something to be added?

CI is failing with messages from saucelabs issues I cannot solve:

  1) Frontend test should open the page and initialise three fragments, each requiring two scripts in internet explorer:
     Error: [sauceJobStatus(false)] Missing auth token.
      at PromiseWebdriver.Webdriver._sauceJobUpdate (node_modules/wd/lib/webdriver.js:186:17)
      at PromiseWebdriver.commands.sauceJobStatus (node_modules/wd/lib/commands.js:2324:24)

[atkinchris]

I think requestFragment needs the initial filterHeaders function. If it's passed in options, it won't be called with filterHeaders - it'll just be assigned over it.

tailor/index.js

Lines 65 to 81 in 571b6d5

	const requestOptions = Object.assign(
	{
	amdLoaderUrl,
	fetchContext: () => Promise.resolve({}),
	fetchTemplate: fetchTemplate(
	templatesPath || path.join(process.cwd(), 'templates')
	),
	fragmentTag: 'fragment',
	handledTags: [],
	handleTag: () => '',
	requestFragment: requestFragment(filterRequestHeaders),
	pipeInstanceName: 'Pipe',
	pipeDefinition: pipeChunk,
	pipeAttributes: getPipeAttributes
	},
	options
	);

If having the implementing application close over filterHeaders is not desired, should there be an additional line of:

options.requestFragment = options.requestFragment(filterRequestHeaders)