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

Jest requires symlinked files twice #3830

Closed
tmeasday opened this issue Jun 15, 2017 · 3 comments
Closed

Jest requires symlinked files twice #3830

tmeasday opened this issue Jun 15, 2017 · 3 comments

Comments

@tmeasday
Copy link

tmeasday commented Jun 15, 2017

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

When requiring a file via a symlink that has already been required directly, Jest executes the file again. This is in contrast to node, which resolves the symlink's path absolutely and thus reuses the original require.

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

Reproduction: https://github.com/tmeasday/jest-symlink-repro

Install, then notice the difference between node index.js and npm test.

What is the expected behavior?

I would expect Jest to behave the same as node here.

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

Tested with jest@20.0.4, node@8.1.0, npm@5.0.3 on OSX 10.12.5

@tmeasday
Copy link
Author

tmeasday commented Jun 22, 2017

Also, a related note--Jest appears to allow packages to require "back" over symlinks as if it is running node --preserve-symlinks: https://github.com/tmeasday/preserve-symlinks-test. Not necessarily a problem, but confusing as it behaves differently to the way your app/package likely does.

gaearon added a commit to gaearon/react that referenced this issue Oct 20, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
@gaearon gaearon mentioned this issue Oct 20, 2017
10 tasks
gaearon added a commit to gaearon/react that referenced this issue Oct 20, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 20, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 23, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 23, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 23, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 24, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 24, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to gaearon/react that referenced this issue Oct 24, 2017
It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.
gaearon added a commit to facebook/react that referenced this issue Oct 24, 2017
* Use relative paths in packages/react

* Use relative paths in packages/react-art

* Use relative paths in packages/react-cs

* Use relative paths in other packages

* Fix as many issues as I can

This uncovered an interesting problem where ./b from package/src/a would resolve to a different instantiation of package/src/b in Jest.

Either this is a showstopper or we can solve it by completely fobbidding remaining /src/.

* Fix all tests

It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.

* Prettier

* Remove @providesModule

* Fix remaining Haste imports I missed earlier

* Fix up paths to reflect new flat structure

* Fix Flow

* Fix CJS and UMD builds

* Fix FB bundles

* Fix RN bundles

* Prettier

* Fix lint

* Fix warning printing and error codes

* Fix buggy return

* Fix lint and Flow

* Use Yarn on CI

* Unbreak Jest

* Fix lint

* Fix aliased originals getting included in DEV

Shouldn't affect correctness (they were ignored) but fixes DEV size regression.

* Record sizes

* Fix weird version in package.json

* Tweak bundle labels

* Get rid of output option by introducing react-dom/server.node

* Reconciler should depend on prop-types

* Update sizes last time
@gaearon
Copy link
Contributor

gaearon commented Oct 25, 2017

#4761

@SimenB SimenB closed this as completed Oct 25, 2017
Ethan-Arrowood pushed a commit to Ethan-Arrowood/react that referenced this issue Dec 8, 2017
* Use relative paths in packages/react

* Use relative paths in packages/react-art

* Use relative paths in packages/react-cs

* Use relative paths in other packages

* Fix as many issues as I can

This uncovered an interesting problem where ./b from package/src/a would resolve to a different instantiation of package/src/b in Jest.

Either this is a showstopper or we can solve it by completely fobbidding remaining /src/.

* Fix all tests

It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.

* Prettier

* Remove @providesModule

* Fix remaining Haste imports I missed earlier

* Fix up paths to reflect new flat structure

* Fix Flow

* Fix CJS and UMD builds

* Fix FB bundles

* Fix RN bundles

* Prettier

* Fix lint

* Fix warning printing and error codes

* Fix buggy return

* Fix lint and Flow

* Use Yarn on CI

* Unbreak Jest

* Fix lint

* Fix aliased originals getting included in DEV

Shouldn't affect correctness (they were ignored) but fixes DEV size regression.

* Record sizes

* Fix weird version in package.json

* Tweak bundle labels

* Get rid of output option by introducing react-dom/server.node

* Reconciler should depend on prop-types

* Update sizes last time
NMinhNguyen pushed a commit to enzymejs/react-shallow-renderer that referenced this issue Jan 29, 2020
* Use relative paths in packages/react

* Use relative paths in packages/react-art

* Use relative paths in packages/react-cs

* Use relative paths in other packages

* Fix as many issues as I can

This uncovered an interesting problem where ./b from package/src/a would resolve to a different instantiation of package/src/b in Jest.

Either this is a showstopper or we can solve it by completely fobbidding remaining /src/.

* Fix all tests

It seems we can't use relative requires in tests anymore. Otherwise Jest becomes confused between real file and symlink.
jestjs/jest#3830

This seems bad... Except that we already *don't* want people to create tests that import individual source files.
All existing cases of us doing so are actually TODOs waiting to be fixed.

So perhaps this requirement isn't too bad because it makes bad code looks bad.

Of course, if we go with this, we'll have to lint against relative requires in tests.
It also makes moving things more painful.

* Prettier

* Remove @providesModule

* Fix remaining Haste imports I missed earlier

* Fix up paths to reflect new flat structure

* Fix Flow

* Fix CJS and UMD builds

* Fix FB bundles

* Fix RN bundles

* Prettier

* Fix lint

* Fix warning printing and error codes

* Fix buggy return

* Fix lint and Flow

* Use Yarn on CI

* Unbreak Jest

* Fix lint

* Fix aliased originals getting included in DEV

Shouldn't affect correctness (they were ignored) but fixes DEV size regression.

* Record sizes

* Fix weird version in package.json

* Tweak bundle labels

* Get rid of output option by introducing react-dom/server.node

* Reconciler should depend on prop-types

* Update sizes last time
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants