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

Load *.{m,c}ts{x} last in node_modules #7259

Merged
merged 12 commits into from
Nov 22, 2023

Conversation

Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Nov 22, 2023

What does this PR do?

Bun currently loads .ts and .tsx files first. This usually works great in local development. This doesn't always work in node_modules because some projects will need special tsconfig.json options which we do not support to work:

  • ts transformer plugins
  • using import {Foo} rather than import {type Foo} or export {Foo} rather than export {type Foo}

This fixes that by only choosing .{m,c}ts{x} first when not in node_modules

I'm marking this as a draft because the logic currently only applies to the main/index file and it needs to also apply to the extension order. We can't merge this until that's fixed. Fixed

Fixes #5426

How did you verify your code works?

There are tests

Copy link
Contributor

github-actions bot commented Nov 22, 2023

@Jarred-Sumner 4 files with test failures on bun-darwin-aarch64:

  • test/integration/next/default-pages-dir/test/dev-server.test.ts
  • test/js/bun/test/test-test.test.ts
  • test/js/bun/util/which.test.ts
  • test/js/web/worker.test.ts

View test output

#674f9d99366aaca4e9900e545ca4ed922a58291d

Copy link
Contributor

github-actions bot commented Nov 22, 2023

@Jarred-Sumner 7 files with test failures on bun-darwin-x64-baseline:

  • test/js/bun/spawn/spawn-streaming-stdout.test.ts
  • test/js/bun/util/which.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/node/watch/fs.watchFile.test.ts
  • test/js/third_party/jsonwebtoken/claim-aud.test.js
  • test/js/third_party/webpack/webpack.test.ts
  • test/js/web/timers/setTimeout.test.js

View test output

#674f9d99366aaca4e9900e545ca4ed922a58291d

Copy link
Contributor

github-actions bot commented Nov 22, 2023

@Jarred-Sumner 5 files with test failures on bun-darwin-x64:

  • test/js/bun/util/which.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/node/watch/fs.watchFile.test.ts
  • test/js/third_party/webpack/webpack.test.ts
  • test/js/web/timers/setTimeout.test.js

View test output

#674f9d99366aaca4e9900e545ca4ed922a58291d

JibranKalia and others added 4 commits November 21, 2023 22:00
* feat(console-log): fix className not printed for objects that are instances of classes

Uses getClassName native method instead of getName

* test(console-log): objects with class names are printed correctly

* test(esbuild): add class name to log message
Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
@Jarred-Sumner Jarred-Sumner marked this pull request as ready for review November 22, 2023 06:00
@Jarred-Sumner Jarred-Sumner merged commit 8106747 into main Nov 22, 2023
15 of 20 checks passed
@Jarred-Sumner Jarred-Sumner deleted the jarred/disable-ts-by-default-in-node-modules branch November 22, 2023 22:11
ryoppippi pushed a commit to ryoppippi/bun that referenced this pull request Feb 1, 2024
* Load `.{m,c}ts{x}` last in node_modules

* feat(console.log): Print className for an object if present (oven-sh#6508)

* feat(console-log): fix className not printed for objects that are instances of classes

Uses getClassName native method instead of getName

* test(console-log): objects with class names are printed correctly

* test(esbuild): add class name to log message

* Fix failing `which` test (oven-sh#7258)

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

* Make test less flaky

* Get it working and add test

* Handle relative paths

* Add comment

* Consolidate + add test

* Bump

* Fix getObjectName

* Update dir_info.zig

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
Co-authored-by: Jibran Kalia <jibran.kalia@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SyntaxError: Indirectly exported binding name 'AnyArena' is not found.
3 participants