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

[TS] DocumentFiles is not exported from _document.tsx #17413

Closed
deadcoder0904 opened this issue Sep 28, 2020 · 6 comments
Closed

[TS] DocumentFiles is not exported from _document.tsx #17413

deadcoder0904 opened this issue Sep 28, 2020 · 6 comments

Comments

@deadcoder0904
Copy link

Feature request

Is your feature request related to a problem? Please describe.

I'm trying to use DocumentFiles like:

getCssLinks({ allFiles }: DocumentFiles) {
	const { assetPrefix } = this.context
	if (!allFiles || allFiles.length === 0) return null

	return allFiles
		.filter((file: any) => /\.css$/.test(file))
		.map((file: any) => (
			<style
				key={file}
				nonce={this.props.nonce}
				data-href={`${assetPrefix}/_next/${file}`}
				dangerouslySetInnerHTML={{
					__html: fs.readFileSync(path.join(process.cwd(), '.next', file), 'utf-8'),
				}}
			/>
		))
}

Describe the solution you'd like

Just have to export DocumentFiles from https://github.com/vercel/next.js/blob/canary/packages/next/pages/_document.tsx#L47

Describe alternatives you've considered

I use any like so:

getCssLinks({ allFiles }: { allFiles: any }) {
	...
}

Additional context

I've written about it here → vercel/next-plugins#238 (comment)

@jsiddiqui
Copy link

Typescript is able to infer the correct type without specifying, due to extending the base class.

More details as vercel/next-plugins#238 (comment)

@deadcoder0904
Copy link
Author

@jsiddiqui Definitely but I get red-squiggly lines in VSCode so I have to use any type instead to get rid of them. Instead of that, why not expose DocumentFiles instead which adds like 1-word export to _document.tsx :)

red-squiggly lines on allFiles

@jsiddiqui
Copy link

One of these should work to strongly type it:

  getCssLinks: Head['getCssLinks'] = ({ allFiles }) => {

OR

  getCssLinks({ allFiles }: Parameters<Head['getCssLinks']>[0]) {

I am not against a one-line export, but I don't see this likely because this whole method is an undocumented hack as you can see from previous comments on the linked issue. This means that we have to manually verify if it works even with every patch release. Adding this one-line export means that the maintainers need to document any change to it in release notes and consider it when evaluating breaking changes.

I'd rather make a case for intrinsic support of inline CSS, e.g. by showing that loading in parallel to JS (multiplexed on http/2), slows down the first contentful paint; rather than asking the maintainers to export an internal type and maintain it.

@deadcoder0904
Copy link
Author

The first one worked fine:

getCssLinks: Head['getCssLinks'] = ({ allFiles }) => {
  ...
}

Personally, I don't see much issue in adding it so I'll still keep this one open to see what others think :)

@timneutkens
Copy link
Member

DocumentFiles is not a public property. Overriding built-in components is not recommended and is likely to break between upgrading Next.js versions.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants