feat(remix-dev): add ability to use tsconfig `paths` aliases other than `~` #2412

F3n67u · 2022-03-20T14:32:58Z

Closes: #2298

Docs
Tests

remix-cla-bot · 2022-03-20T14:33:01Z

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

remix-cla-bot · 2022-03-20T14:34:30Z

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

machour · 2022-03-20T15:22:37Z

@F3n67u thank you for taking the time for this 🙌🏼
could you just rebase it against the dev branch ?

westmark · 2022-03-21T11:35:08Z

Just wondering, will this perhaps solve #1405?

F3n67u · 2022-03-21T12:51:17Z

@F3n67u thank you for taking the time for this 🙌🏼 could you just rebase it against the dev branch ?

@machour this branch is branched out by main, after rebase against dev branch, the commit history is a mess. should I branch out by dev instead of main and merge this pr into dev branch?

machour · 2022-03-21T12:58:16Z

Yes please 🙏🏼
Changes to code goes to dev

F3n67u · 2022-03-21T13:15:17Z

Just wondering, will this perhaps solve #1405?

@westmark I don't know much background about #1405. This pr will probably solve the following limit which lists in #1405;

Remix currently treats modules not prefixed with . or ~ as external, so they are not bundled at compile time (https://github.com/remix-run/remix/blob/7d1dda3/packages/remix-dev/compiler.ts#L411-L413).

machour · 2022-03-21T13:29:37Z

@F3n67u make sure to unmark the draft status when the PR is ready please.

F3n67u · 2022-03-21T13:53:51Z

@F3n67u make sure to unmark the draft status when the PR is ready please.

@machour ready now. I added an integration test.

kiliman · 2022-03-21T14:47:40Z

Do you know if tsconfig-paths also support JS projects that use jsconfig instead?

F3n67u · 2022-03-21T15:00:31Z

Do you know if tsconfig-paths also support JS projects that use jsconfig instead?

I think no, see this issue dividab/tsconfig-paths#127

ryanflorence · 2022-03-21T19:41:16Z

I generally dislike being able to change what an import path means, but because nested route folders make importing super annoying we added just one alias to the root with ~ for convenience.

Looking at what most people do w/ these import aliases, it's usually just pointing to some root folder anyway:

import from "components/button"
import from "~/components/button"

I think I could be convinced for the migration use case--but even then, most apps migrating to remix could do a find/replace in probably half an hour to switch to ~.

I really don't want to look at a path in a remix app and have no clue what it points to.

mjackson · 2022-03-22T00:03:05Z

I really don't want to look at a path in a remix app and have no clue what it points to.

I hear what you're saying @ryanflorence, but I also think there's a strong case for including this behavior in Remix since tsconfig.json already supports the paths option. As long as we're just piggybacking on top of that (i.e. not introducing some separate config in remix.config.js) I'd support this change. TypeScript supports it. We're just following their lead here.

westmark · 2022-03-22T08:41:55Z

I hear what you're saying @ryanflorence, but I also think there's a strong case for including this behavior in Remix since tsconfig.json already supports the paths option. As long as we're just piggybacking on top of that (i.e. not introducing some separate config in remix.config.js) I'd support this change. TypeScript supports it. We're just following their lead here.

Not to mention that this change will allow NX to fully support Remix(nrwl/nx#8019), unless I'm sadly mistaken. But yeah, I also generally agree with what @ryanflorence is saying.

F3n67u · 2022-03-22T09:24:05Z

@machour test on ci takes forever to run, now it takes 3h+. could you help to take a look at it?

machour · 2022-03-22T12:08:03Z

@F3n67u I tried to cancel it, but something weird is going on with this build.. can you try to nudge it by doing a new commit?

machour · 2022-03-22T12:10:44Z

@ryanflorence Depending on the size of the project, having aliases may come handy for some teams.
I see no harm in leaving the possibility open for developers to define their own aliases.

F3n67u · 2022-03-22T12:18:25Z

@F3n67u I tried to cancel it, but something weird is going on with this build.. can you try to nudge it by doing a new commit?

@machour do you mean I push a new empty commit so to rerun the ci ?

machour · 2022-03-22T12:20:04Z

@F3n67u never mind, the build process finally got canceled after my message. I just triggered a new one, hopefully this one won't get stuck

integration/path-mapping-test.ts

packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts

machour · 2022-03-23T09:27:34Z

@F3n67u I think something is definitely going wrong with the CI when running with this PR changes.
This needs to be properly investigated, I'm available on discord if you need me to cancel builds while you do that.

mcansh · 2022-03-31T18:10:34Z

this is looking great so far @F3n67u, we just need to add support for MDX,

remix/packages/remix-dev/compiler/plugins/mdx.ts

Lines 19 to 24 in eafad35

    
           return { 
        
             path: args.path.startsWith("~/") 
        
               ? path.resolve(config.appDirectory, args.path.replace(/^~\//, "")) 
        
               : path.resolve(args.resolveDir, args.path), 
        
             namespace: "mdx", 
        
           };

- gonna take a few minutes to see if i can get it real quick, but you probably know more about tsconfig-paths than I do

also strip indents from integration files before writing them to disk Signed-off-by: Logan McAnsh <logan@mcan.sh>

Signed-off-by: Logan McAnsh <logan@mcan.sh>

mcansh · 2022-03-31T20:21:41Z

tests are failing on dev at the moment which is why they are failing here

github-actions · 2022-04-06T15:32:00Z

🤖 Hello there,

We just published version v1.3.5-pre.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

npirotte · 2022-04-07T08:55:03Z

I there, I just tried v1.3.5-pre.1, it works fine, except of a detail:

Context:
I'm running the app using nx with @nrwl/remix plugin.

So in a nutshell, all dependencies are declared in the root package.json, but remix commands are declared in the app specific package.json.

When I dev or build my application, I have a warning for every dependency that I import, because remix doesn't find them in the app specific package.json

The path "react-dom/server" is imported in app/entry.server.tsx but react-dom is not listed in your package.json dependencies. Did you forget to install it?
The path "react" is imported in app/routes/demos/actions.tsx but react is not listed in your package.json dependencies. Did you forget to install it?
The path "@react-hook/size" is imported in ../bar-chart/src/lib/bar-chart.tsx but @react-hook/size is not listed in your package.json dependencies. Did you forget to install it?
The path "@visx/scale" is imported in ../bar-chart/src/lib/bar-chart.tsx but @visx/scale is not listed in your package.json dependencies. Did you forget to install it?
The path "@ngneat/falso" is imported in ../bar-chart/src/lib/bar-chart.tsx but @ngneat/falso is not listed in your package.json dependencies. Did you forget to install it?
The path "@visx/group" is imported in ../bar-chart/src/lib/bar-chart.tsx but @visx/group is not listed in your package.json dependencies. Did you forget to install it?
The path "@visx/shape" is imported in ../bar-chart/src/lib/bar-chart.tsx but @visx/shape is not listed in your package.json dependencies. Did you forget to install it?
The path "lodash/fp" is imported in ../bar-chart/src/lib/bar-chart.tsx but lodash is not listed in your package.json dependencies. Did you forget to install it?
The path "@visx/axis" is imported in ../bar-chart/src/lib/bar-chart.tsx but @visx/axis is not listed in your package.json dependencies. Did you forget to install it?
Built in 357ms

The list is quite OK for now because that's a sandbox project, but I can imagine the list growing a lot for a real world project :)

I don't know where I should create an issue for that. is it remix responsibility, nrwl responsibility?

machour · 2022-04-07T09:02:34Z

@npirotte I'm not sure what this have to do with this PR.
Please use discussions to ask for help, or fill in a new issue if you think this is a Remix problem

jacargentina · 2022-04-08T21:54:40Z

I think this PR maybe the culprit; just updated to 1.3.5, and getting lots of errors.

The ~ imports seem not be working anymore.

kiliman · 2022-04-08T21:55:59Z

Yes, I'm getting similar errors. I had to roll back to 1.3.4 to continue working.

F3n67u · 2022-04-09T00:28:40Z

I think this PR maybe the culprit; just updated to 1.3.5, and getting lots of errors.

The ~ imports seem not be working anymore.

@jacargentina would you mind fill a new issue witha reproduction repo?

jacargentina · 2022-04-10T19:54:25Z

@F3n67u I think there is some change on remix related to importing auth.server.ts on root.tsx

I've commented that import, so the first error is gone, but now I have another one: app/routes/bookeditor.$id.tsx is importing import { clientFn } from '~/apollo.server'; (note the .server), gives me the very same error

Maybe I must adapt my code to some change on remix about those .server files ?

F3n67u · 2022-04-11T00:37:24Z

@F3n67u I think there is some change on remix related to importing auth.server.ts on root.tsx

I've commented that import, so the first error is gone, but now I have another one: app/routes/bookeditor.$id.tsx is importing import { clientFn } from '~/apollo.server'; (note the .server), gives me the very same error

Maybe I must adapt my code to some change on remix about those .server files ?

@jacargentina what's inside your tsconfig.json or jsconfig.json?

jacargentina · 2022-04-12T12:01:26Z

@F3n67u

My tsconfig.json:

{
  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
  "compilerOptions": {
    "module": "esnext",
    "lib": ["DOM", "DOM.Iterable", "ES2019"],
    "esModuleInterop": true,
    "jsx": "react-jsx",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "strict": true,
    "paths": {
      "~/*": ["./app/*"]
    },

    // Remix takes care of building everything in `remix build`.
    "noEmit": true
  }
}

I've adjuted it like this, and now it is working; I basically removed "module" and added "target"

{
  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
  "compilerOptions": {
    "lib": ["DOM", "DOM.Iterable", "ES2019"],
    "isolatedModules": true,
    "esModuleInterop": true,
    "jsx": "react-jsx",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "target": "ES2019",
    "strict": true,
    "baseUrl": ".",
    "paths": {
      "~/*": ["./app/*"]
    },

    // Remix takes care of building everything in `remix build`.
    "noEmit": true
  }
}

mcansh · 2022-04-13T20:04:34Z

I've adjuted it like this, and now it is working; I basically removed "module" and added "target"

you also added a baseUrl which after debugging one of my apps, is the kicker to make the aliases all work - gonna add some better error messaging for that :)

MichaelDeBoey · 2022-04-19T11:34:42Z

@F3n67u dividab/tsconfig-paths#199 is now merged.
Do you want to create a PR that uses that new version + removes the temporary workaround you introduced in this PR?
Also make sure to remove unnecessary dependencies that were introduced for the workaround.

F3n67u · 2022-04-19T11:39:51Z

@F3n67u dividab/tsconfig-paths#199 is now merged. Do you want to create a PR that uses that new version + removes the temporary workaround you introduced in this PR? Also make sure to remove unnecessary dependencies that were introduced for the workaround.

@MichaelDeBoey will do it after the new version is published! tsconfig-paths have not released new version yet.

MichaelDeBoey · 2022-05-03T12:12:07Z

@F3n67u tsconfig-paths v4.0.0 is now released

https://github.com/dividab/tsconfig-paths/blob/master/CHANGELOG.md#400---2022-05-02

F3n67u · 2022-05-03T13:09:38Z

@F3n67u tsconfig-paths v4.0.0 is now released

https://github.com/dividab/tsconfig-paths/blob/master/CHANGELOG.md#400---2022-05-02

great. I will make a pr later.

…an `~` (remix-run#2412) Co-authored-by: Jacob Ebey <jacob.ebey@live.com> Co-authored-by: Logan McAnsh <logan@mcan.sh>

remix-cla-bot bot added the CLA Signed label Mar 20, 2022

machour added the feat:dx Issues related to the developer experience label Mar 20, 2022

F3n67u force-pushed the fix/paths branch from d05bd77 to 4bbc2af Compare March 21, 2022 12:47

F3n67u force-pushed the fix/paths branch from 4bbc2af to b0840ec Compare March 21, 2022 13:09

F3n67u changed the base branch from main to dev March 21, 2022 13:10

F3n67u marked this pull request as ready for review March 21, 2022 13:52

MichaelDeBoey requested changes Mar 22, 2022

View reviewed changes

integration/path-mapping-test.ts Outdated Show resolved Hide resolved

packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts Show resolved Hide resolved

MichaelDeBoey changed the title ~~fix: can't use tsconfig 'paths' aliases other than ~~~ fix(remix-dev): add ability to use tsconfig paths aliases other than ~ Mar 22, 2022

MichaelDeBoey added the package:dev label Mar 22, 2022

F3n67u force-pushed the fix/paths branch from 4c04e4f to bd0c5cf Compare March 23, 2022 13:34

Merge branch 'dev' into fix/paths

9223901

mcansh added 4 commits March 31, 2022 15:48

feat: add path-mapping support for mdx files

136aa8e

also strip indents from integration files before writing them to disk Signed-off-by: Logan McAnsh <logan@mcan.sh>

chore: update import to use alias

b9996e2

Signed-off-by: Logan McAnsh <logan@mcan.sh>

chore: move createMatchPath to avoid importing cross-plugin

1bfa104

Signed-off-by: Logan McAnsh <logan@mcan.sh>

Merge branch 'dev' into fix/paths

2199011

mcansh changed the title ~~fix(remix-dev): add ability to use tsconfig paths aliases other than ~~~ feat(remix-dev): add ability to use tsconfig paths aliases other than ~ Mar 31, 2022

mcansh merged commit 7c935a4 into remix-run:dev Mar 31, 2022

machour mentioned this pull request Mar 31, 2022

Can't use other 'paths' aliases in typescript #2298

Closed

F3n67u deleted the fix/paths branch April 1, 2022 00:20

machour mentioned this pull request Apr 12, 2022

[Feature]: Custom package path resolver #1405

Closed

Girish21 mentioned this pull request Apr 27, 2022

Error on npm run dev Girish21/speed-metal-stack#4

Closed

MichaelDeBoey mentioned this pull request May 3, 2022

refactor(remix-dev): clean up custom tsconfig loader #3074

Merged

ilijaNL mentioned this pull request Nov 8, 2023

Remix compiler always bundles tsconfig path aliases #7939

Open

1 task

feat(remix-dev): add ability to use tsconfig paths aliases other than ~ #2412

feat(remix-dev): add ability to use tsconfig paths aliases other than ~ #2412

Conversation

F3n67u commented Mar 20, 2022 • edited

remix-cla-bot bot commented Mar 20, 2022

remix-cla-bot bot commented Mar 20, 2022

machour commented Mar 20, 2022

westmark commented Mar 21, 2022

F3n67u commented Mar 21, 2022 • edited

machour commented Mar 21, 2022

F3n67u commented Mar 21, 2022

machour commented Mar 21, 2022

F3n67u commented Mar 21, 2022

kiliman commented Mar 21, 2022

F3n67u commented Mar 21, 2022

ryanflorence commented Mar 21, 2022 • edited

mjackson commented Mar 22, 2022

westmark commented Mar 22, 2022

F3n67u commented Mar 22, 2022

machour commented Mar 22, 2022

machour commented Mar 22, 2022

F3n67u commented Mar 22, 2022 • edited

machour commented Mar 22, 2022

machour commented Mar 23, 2022

mcansh commented Mar 31, 2022

mcansh commented Mar 31, 2022

github-actions bot commented Apr 6, 2022 • edited by mcansh

npirotte commented Apr 7, 2022

machour commented Apr 7, 2022

jacargentina commented Apr 8, 2022

kiliman commented Apr 8, 2022

F3n67u commented Apr 9, 2022

jacargentina commented Apr 10, 2022 • edited

F3n67u commented Apr 11, 2022 • edited

jacargentina commented Apr 12, 2022

mcansh commented Apr 13, 2022 • edited

MichaelDeBoey commented Apr 19, 2022

F3n67u commented Apr 19, 2022

MichaelDeBoey commented May 3, 2022

F3n67u commented May 3, 2022 • edited

feat(remix-dev): add ability to use tsconfig `paths` aliases other than `~` #2412

feat(remix-dev): add ability to use tsconfig `paths` aliases other than `~` #2412

F3n67u commented Mar 20, 2022 •

edited

F3n67u commented Mar 21, 2022 •

edited

ryanflorence commented Mar 21, 2022 •

edited

F3n67u commented Mar 22, 2022 •

edited

github-actions bot commented Apr 6, 2022 •

edited by mcansh

jacargentina commented Apr 10, 2022 •

edited

F3n67u commented Apr 11, 2022 •

edited

mcansh commented Apr 13, 2022 •

edited

F3n67u commented May 3, 2022 •

edited