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

Directly export all components in next.js code as Component.tsx files #1554

Closed
humphd opened this issue Jan 18, 2021 · 3 comments · Fixed by #1667
Closed

Directly export all components in next.js code as Component.tsx files #1554

humphd opened this issue Jan 18, 2021 · 3 comments · Fixed by #1667
Assignees
Labels
area: front-end area: nextjs Nextjs related issues developer experience Helping the Developer Experience type: enhancement New feature or request
Milestone

Comments

@humphd
Copy link
Contributor

humphd commented Jan 18, 2021

Our current Gastby code uses a component file layout like this:

/Foo
    index.js    <---- takes care of combining the JSX and CSS, and provides the default export for Foo
    Foo.jsx
    Foo.css

I want to move toward this:

/components
    Foo.tsx

Some history and thoughts on the various approaches to this:

  • JSX (and for TypeScript, TSX) is a non-standard extension of JavaScript
  • Some people, especially outside the React ecosystem, don't like using .js for a file that isn't standard JavaScript (or .ts for TypeScript).
  • Transpilers (i.e., Babel, which translates JSX to pure JS) doesn't care what you call it, and at runtime, there is no-such thing as JSX: it all gets converted to pure functions.
  • The general advice with transpiled code is to import it without the extension: import Foo from './foo' without any extension, and let the module system figure it out at build- vs. run-time.
  • We are using React and next.js, so the vast majority of files are likely to contain JSX (i.e., it's not an edge case we need to worry about).

I think that another reason we used the index.js approach to do the final export was that we also wanted to put CSS files in the same scope:

Later, we moved to Material UI, and with it, CSS-in-JS. Next.js doesn't really like global CSS anyway (they either want you to use CSS Modules or CSS-in-JS).

In summary, I think we can ditch the index.js export files completely. If we follow the patterns in the next.js TypeScript example, in this PR I would suggest we do this:

src/frontend/next/src/components/BackToTopButton/BackToTopButton.tsx becomes src/frontend/next/src/components/BackToTopButton.tsx (i.e., drop the directory and use tsx).

Let's do this for all of our next.js code once it's in. If you're reviewing new code, you can ask the author to do it now, or leave it and we'll do it all at once later.

@humphd humphd added type: enhancement New feature or request area: front-end developer experience Helping the Developer Experience area: nextjs Nextjs related issues labels Jan 18, 2021
@tonyvugithub
Copy link
Contributor

@humphd : Should we just remove the index.ts and keep the folder though? Some of the components with multiple dependent will require the folder to have all relating things in one place.

@humphd
Copy link
Contributor Author

humphd commented Jan 18, 2021

If you are importing Foo/index or Foo both ways will work.

I don't think a folder is needed if you don't have any secondary files. I would use a folder where you have a set of files, and an index.ts help manage the default entry point.

@HyperTHD
Copy link
Contributor

HyperTHD commented Feb 6, 2021

#1667 is the last issue related with directly exporting the components. It's also the last component to be ported over to next. Once that lands, this issue can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: front-end area: nextjs Nextjs related issues developer experience Helping the Developer Experience type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants