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

Trace .tsx input files #40

Merged
merged 6 commits into from Aug 12, 2019
Merged

Trace .tsx input files #40

merged 6 commits into from Aug 12, 2019

Conversation

tylerc
Copy link
Contributor

@tylerc tylerc commented Aug 9, 2019

This fixes #39 the issue I just filed around .tsx files being unable to be used as the entrypoint for a route.

I noticed in @now/node this callback was responsible for compiling my .ts files, but it was never getting called for my .tsx files: https://github.com/zeit/now-builders/blob/canary/packages/now-node/src/index.ts#L177

This pull request makes node-file-trace aware that .tsx is a valid file extension for TypeScript files.

@tylerc tylerc requested review from lucleray and styfle as code owners Aug 9, 2019
@tylerc
Copy link
Contributor Author

tylerc commented Aug 9, 2019

Pinging @styfle here as well.

src/node-file-trace.js Show resolved Hide resolved
@tylerc
Copy link
Contributor Author

tylerc commented Aug 9, 2019

@styfle unit tests have been pushed. Since it didn't quite fit the mold of the other tests, I had to add another call to it() in unit.test.js, hopefully that's okay.

I also found a couple other places that referenced .ts but not .tsx, so I fixed those up as well.

I couldn't figure out how to test the one in analyze.js (nothing ever seemed to get to that code) but both changes in node-file-trace.js are covered inthe unit test I added.

test/unit/tsx2/output.js Outdated Show resolved Hide resolved
test/unit/tsx2/output.js Outdated Show resolved Hide resolved
test/unit/tsx2/input.tsx Outdated Show resolved Hide resolved
test/unit.test.js Outdated Show resolved Hide resolved
@tylerc
Copy link
Contributor Author

tylerc commented Aug 12, 2019

@styfle All your suggestions should now be integrated.

@styfle styfle self-assigned this Aug 12, 2019
@styfle styfle changed the title Ensure .tsx files also get compiled. Trace .tsx input files Aug 12, 2019
@styfle styfle added the automerge Automatically merge PR once checks pass label Aug 12, 2019
styfle
styfle approved these changes Aug 12, 2019
Copy link
Member

@styfle styfle left a comment

Amazing work, thanks! 🤩

@tylerc
Copy link
Contributor Author

tylerc commented Aug 12, 2019

Thank you!

@kodiakhq kodiakhq bot merged commit 7444f9e into vercel:master Aug 12, 2019
styfle added a commit that referenced this pull request Aug 22, 2022
This PR changes the behavior of dependency tracing such that `fs.readFile('./file.js')` is no longer considered a `dependency` and it will not be traced. Instead, it is considered an `asset` the same way a `fs.readFile('./file.txt')` is. The `dependency` type will be reserved for `import` and `require` only.

- Fixes #303 
- Related to vercel/customer-issues#608

Note that this could be considered a breaking change since #40 expected the `readFile()`  config to be called for assets but it should only be called for dependencies.

In order to reduce the amount of breakage, `require.resolve()` was left as dependency instead of asset since its often passed to `child_process.spawn()` which needs the dependencies.

Co-authored-by: JJ Kasper <jj@jjsweb.site>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge PR once checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.tsx files used as an entrypoint for an API fail to get compiled
2 participants