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

Modules resolved absolutely against src are treated as external: support TS Paths aliases #91

Open
a-type opened this issue May 8, 2019 · 34 comments
Labels
kind: feature New feature or request scope: upstream Issue in upstream dependency solution: workaround available There is a workaround available for this issue topic: TS Paths Aliases Related to using aliases with TypeScript paths

Comments

@a-type
Copy link
Contributor

a-type commented May 8, 2019

Followup to #89 and #87.

Current Behavior

Running tsdx create initializes a tsconfig.json with absolute module resolution relative to the src directory. I have VS Code configured to prefer absolute paths if possible when importing modules, so if I have a module like...

src/foo.ts

export const foo = () => 'bar';

And I type

foo()

and hit Tab, VS Code will add the following import:

import { foo } from 'foo';

This is valid and properly resolved by Typescript. However, if I compile my library, the current Rollup behavior assumes that foo is an external module and doesn't include it in the bundle.

Expected behavior

I would expect my local modules to be included in my bundle regardless of whether I imported them using 'absolute' resolution as above,

OR

I would expect tsdx to not initialize tsconfig.json with resolution rules which would lead me to believe importing modules relative to src is a valid thing to do.

Suggested solution(s)

Either fix the external modules rules to properly understand local src-based resolution, or remove the default src resolution rules from tsconfig.json to avoid confusing users.

Additional context

I probably hit this more often than most because I have configured my editor to prefer absolute path resolutions if available.

Your environment

Software Version(s)
TSDX 0.5.7
TypeScript 3.4.3
Browser N/A
npm/Yarn NPM 6.2.0
Operating System Windows 10
@swyxio
Copy link
Collaborator

swyxio commented May 8, 2019

great issue. we should probably resolve this on a Rollup level. thanks for writing it up. i'll take a look at the rollup config but am not sure how to solve it

@swyxio
Copy link
Collaborator

swyxio commented May 8, 2019

@swyxio
Copy link
Collaborator

swyxio commented May 8, 2019

probably want to add it here. want to try it in a local version and PR?

@jaredpalmer
Copy link
Owner

I defer to you @sw-yx

@a-type
Copy link
Contributor Author

a-type commented May 9, 2019

Continuing down this rabbit-hole: The potential issue I see with aliasing is that the tsconfig is bootstrapped into the user's controlled files, but the rollup config is still controlled by tsdx. If the src relative resolution is hard-coded into tsdx's rollup config, you may encounter a scenario where the user has a legitimate reason to remove the resolution from tsconfig.json, but cannot also remove it from Rollup. For instance, if there was a name conflict with an NPM module and they decided to ditch the src resolution.

So it would seem that we might want to detect if the user has modified their module resolution rules in tsconfig.json, or prevent them from doing so, or at least warn them of the behavior if they do.

It's kind of a headache that TS module resolution is decoupled from the actual module resolution used to build the bundle, and the two are split between user control and tsdx internals. It's confusing even in normal TS development... if only the rules set in tsconfig.json could also just automagically control how a final bundle is assembled.

@swyxio
Copy link
Collaborator

swyxio commented May 9, 2019

we could handroll (or publish) a plugin to read tsconfig.json... the source doesnt look too bad? basic or we could fork the alias plugin @a-type

@jantimon
Copy link

jantimon commented May 12, 2019

Please keep the current behaviour.

I have muliple small tsdx packages inside a mono repository which look like this (names simplified):

├── core
└── utils
    ├── util1
    ├── util2
    └── util3

Now when published to npm it is important that util1 does not inline core but uses it as an external.

So it is really important for me that import { core } from 'core'; is used as an external although the files are not coming from a node_modules folder.

@swyxio
Copy link
Collaborator

swyxio commented May 12, 2019

@jantimon doesnt that just mean core should be marked as a peer dependency? we can make sure to respect that too.

@jantimon
Copy link

Yes core is a peer dependency

@jaredpalmer
Copy link
Owner

Should we make an external flag where you can hand write externals?

@mattoni
Copy link

mattoni commented May 30, 2019

I'd like to add to this, that using absolute paths for imports is causing resolution issues in my generated declaration files. My index.ts looks like this:

import * as Auth from "./auth";
import * as Request from "./common/api/request";
import * as Errors from "./common/api/error";
import { QueryParams } from "./common/api/query";
import { Settings } from "./common/api/settings";
import * as Structs from "./common/structs";
import * as Notifications from "./notifications";
import * as Account from "./resources/accounts";
import * as ChangeLog from "./resources/changelog";
import * as Hubs from "./resources/hubs";
import * as Billing from "./resources/billing";
import * as Containers from "./resources/containers";
import * as Infrastructure from "./resources/infrastructure";
import * as Stacks from "./resources/stacks";
import * as Images from "./resources/images";
import * as DNS from "./resources/dns";
import * as Environments from "./resources/environments";
import * as Jobs from "./resources/jobs";
import * as Secrets from "./resources/secrets";
import * as Projects from "./resources/projects";
export { Capability } from "./resources/hubs";
export { Auth, ChangeLog, Request, Errors, QueryParams, Settings, Structs, Account, Hubs, Billing, Containers, Infrastructure, Stacks, Images, DNS, Environments, Jobs, Secrets, Projects, Notifications, };

and in my declaration files I'm getting stuff like:

image

@mattoni
Copy link

mattoni commented Jun 20, 2019

Just checking in to see if there is a solution to the absolute pathing that I'm missing, or if this issue is truly related. Would love to use tsdx, it's extremely valuable.

@RJahnavi
Copy link

Not sure if the below issue is related to this thread or not:
Rollup changes import of absolute path to relative path while bundling

I am importing a absolute path in some files of my repo and try to bundle it with rollup
My import where /api/ is absolute path:
import * from '/api/myFile.js'

But when I bundle it, rollup changes it to relative path and it looks like:
import * from from '../../../../api/myFile.js'

And the above path doesn't exist in my app :(

My rollup config:
rollup src\input.js -o lib\bundle.js -f esm --inlineDynamicImports=true

I tried making '/api/' path as external, that didn't change anything. And i tried using few rollup plugins - includepaths, root-import,etc nothing worked
Is there any solution to this ?

@swyxio
Copy link
Collaborator

swyxio commented Jul 17, 2019

no idea tbh. not spending any time on this right now. i'll return to this once i'm in build mode. happy to take a look at PRs

@export-mike
Copy link

@sw-yx @jaredpalmer any examples on a mono repo with tsdx setup?

@swyxio
Copy link
Collaborator

swyxio commented Aug 29, 2019

no. check the other issues for other mono repo folks. we can maybe create an umbrella issue to look holistically at monorepo issues and appoint a maintainer to look after that bc i dont think either jared or i spend that much time in monorepo land.

@arvigeus
Copy link

arvigeus commented Dec 11, 2019

Not sure if this is a separate issue, or the same problem: Absolute imports does not work. Repo to reproduce: https://github.com/arvigeus/tsdx-broken-lib
Tried the suggested solutions, didn't worked

@swyxio
Copy link
Collaborator

swyxio commented Dec 12, 2019

yeah there've been a few issues on this too. havent spent much time on it. sidenote tho - monorepo support is about to get a lot better since jared moved formik to monorepo

@mattoni
Copy link

mattoni commented Feb 11, 2020

Though I've gotten the relative paths to build, the typings are all messed up so I'm getting "any" for any type that's imported absolutely when trying to use the lib in production.

The definition file that was generated by tsdx (after configuring babel per the HOWTO guide on setting up absolute imports)

image

and attempting to use the getRequest() function which relies on a definition for an absolute import:

image

@zprjk
Copy link

zprjk commented Mar 8, 2020

I've wasted a day to make import absolute paths work. If you reading this I hope you don't make the same mistakes I did. No an expert but I got it working as follow:

-- Just an example --

// Say this is a React UI lib
./src
	/components/button.tsx
  	/components/..
  	/index.tsx // export {default as Button} from './components/button';
    /utils/math.tsx
    /utils/string.tsx
./test
  	/button.test.tsx
// tsconfig.json
{
  // ..
  "compilerOptions" : {
   // If you edit here you ALSO have to edit `.babelrc.js` and `jest.config.js`
   "paths": {
     // "*": ["src/*", "node_modules/*"] // "*": "src/*" causes much trouble than it tries to solve.
     "*": ["node_modules/*"] 
     "#comps": ["src/*"],
     "#utils/*": ['src/utils/*'],
    },
  }
}
//.babelrc.js
module.exports = {
  // ..
  "plugins": [
    [
	  // npm i -D babel-plugin-module-resolver
      // Alias resolver for `tsconfig.paths`
      // If you edit `tsconfig.json` paths you have to edit also here.
      "module-resolver",
      {
        "extensions": ['.ts', '.tsx'], // not sure if you really need this
        "root": "./",
        "alias": {
          // similar declaration as tsconfig.json but it's actually different !!
          "#comps": "./src", // note that './' is important here.
          '#utils': ['./src/utils'],
          
          // "*": ["./src/*"] 
          // "#utils/*": ['./src/utils/*'],
          // ☝️ Any path with "*" won't work as you expect for module-resolver 
          // since it treats as a character instead of a glob pattern. 
          // Wasted a day because of this stupid mistake.
        }
      }
    ]
  ]
}
// jest.config.js
module.exports = {
  //..

  // Alias resolver for `tsconfig.paths` in Jest
  // If you edit `tsconfig.json` paths you also have to edit here.
  moduleNameMapper: {
    // https://kulshekhar.github.io/ts-jest/user/config/#paths-mapping
    '^#comps$': '<rootDir>/src',
    '^#utils/(.*)$': '<rootDir>/src/utils/$1',
  },
}
// > src/tests/button.test.tsx <
// works fine with vscode and when you run `npm test`
import {Button} from '#comps';
import math from '#utils/math';
// ..

// > src/some/deep/dir <
// works fine with vscode and when you run `npm start/run build`
import {Button} from '#comps';
import math from '#utils/math';
// ..

The comment #379 (comment) (require('./tsconfig.json').compilerOptions.paths) made me confuse how babel-plugin-module-resolver was suppose to work.

I still have to test with storybook and make sure everything works.

@agilgur5 agilgur5 changed the title Modules resolved absolutely against src are treated as external Modules resolved absolutely against src are treated as external: support TS Paths aliases Mar 9, 2020
@agilgur5 agilgur5 added the topic: TS Paths Aliases Related to using aliases with TypeScript paths label Mar 10, 2020
@chrbala
Copy link

chrbala commented Mar 12, 2020

I'm interested in getting a solution for this getting shipped into TSDX.

  1. Is there alignment here on how exactly this should work as a first class TSDX feature?
  2. Is there currently work being done to get this added to TSDX itself? It seems like a lot of people are dropping solutions they've made by extending the base TSDX config, but is anyone planning to work on a PR?

@chrbala
Copy link

chrbala commented Mar 12, 2020

That's a really great synopsis of the situation - thank you! Given all of that, over the coming weeks, I may do some research into this and will report findings back here. It's not directly on our roadmap at the moment, so it may not be super timely, but we can make a bit of room for this.

@jantimon
Copy link

I guess this would rather be an anti-pattern for library development as users of the library might have to adjust their tsconfig to use the library sources

@agilgur5
Copy link
Collaborator

@jantimon you're correct if there were no additional transforms -- but a portion of this issue and the surrounding ones are around adding such a transform so that one can use paths aliases beyond their original intent (which much of the TS community wants to be able to do, per my above links)

@chmac
Copy link

chmac commented May 28, 2020

Workaround

To stop vscode using absolute imports when it auto imports, and thus my builds being broken, I was able to add the following to my workspace file:

"settings": {
    "typescript.preferences.importModuleSpecifier": "relative"
  }

chmac added a commit to GenerousLabs/plans that referenced this issue May 28, 2020
@vkondamu
Copy link

I've worked on a solution that uses absolute paths using @/ with tsdx, and made sure that these modules are not treated as external in tsdx-absolute-paths. If you're interested in making this an out-of-the-box feature with tsdx, what would be the next steps to get this shipped?

@makker
Copy link

makker commented Jun 11, 2020

tscpaths can also be used to change the absolute paths to relative paths for your build.

@pedroapfilho

This comment has been minimized.

@agilgur5 agilgur5 added kind: feature New feature or request solution: workaround available There is a workaround available for this issue labels Aug 25, 2020
@agilgur5
Copy link
Collaborator

agilgur5 commented Aug 25, 2020

@pedroapfilho the buggy default configuration has been removed from the templates as of v0.13.3, which resolves this issue's "OR" expected behavior.

I've written above in detail and with many links (which also link out) about this situation, which is not TSDX-specific and isn't a TS bug either, but a feature many in the TS community would like which TS itself has rejected.

I've started work on a separate Babel plugin to implement this behavior (which was my recommendation beyond the various workarounds), but I haven't had time to finish it up and test it.

jasonraimondi added a commit to jasonraimondi/ts-oauth2-server that referenced this issue Oct 4, 2020
I've tried everything to get absolute imports working, unfortunately they
were more of a pain than they were worth. Maybe one day there will be
better support. For now, they were causing problems for end users

see this issue: jaredpalmer/tsdx#91
@agilgur5 agilgur5 added the scope: upstream Issue in upstream dependency label Oct 22, 2020
@ankit
Copy link

ankit commented Jan 5, 2021

Since using babel-plugin-module-resolver does not actually correctly transform the generated *.d.ts, I was able to create a workaround (customizing tsdx.config.js) to use ts-transform-paths and ttypescript

// https://github.com/zerkalica/zerollup/tree/master/packages/ts-transform-paths
const ttypescript = require('ttypescript');
const typescript = require('rollup-plugin-typescript2');

module.exports = {
  rollup(config, options) {
    const rpt2Plugin = config.plugins.find(p => p.name === 'rpt2');
    const rpt2PluginIndex = config.plugins.indexOf(rpt2Plugin);

    const tsconfigPath = options.tsconfig || 'tsconfig.json';

    // borrowed from https://github.com/facebook/create-react-app/pull/7248
    const tsconfigJSON = ttypescript.readConfigFile(
      tsconfigPath,
      ttypescript.sys.readFile
    ).config;

    const tsCompilerOptions = ttypescript.parseJsonConfigFileContent(
      tsconfigJSON,
      ttypescript.sys,
      './'
    ).options;

    const customRPT2Plugin = typescript({
      typescript: ttypescript,
      tsconfig: options.tsconfig,
      tsconfigDefaults: {
        exclude: [
          // all TS test files, regardless whether co-located or in test/ etc
          '**/*.spec.ts',
          '**/*.test.ts',
          '**/*.spec.tsx',
          '**/*.test.tsx',
          // TS defaults below
          'node_modules',
          'bower_components',
          'jspm_packages',
          'dist',
        ],
        compilerOptions: {
          sourceMap: true,
          declaration: true,
          jsx: 'react',
        },
      },
      tsconfigOverride: {
        compilerOptions: {
          // TS -> esnext, then leave the rest to babel-preset-env
          target: 'esnext',
          // don't output declarations more than once
          ...(!options.writeMeta
            ? { declaration: false, declarationMap: false }
            : {}),
        },
      },
      check: !options.transpileOnly && options.writeMeta,
      useTsconfigDeclarationDir: Boolean(
        tsCompilerOptions && tsCompilerOptions.declarationDir
      ),
    });

    config.plugins.splice(rpt2PluginIndex, 1, customRPT2Plugin);
    return config;
  },
};

I am still debating if this is maintainable (I do really like the aliases), but thought I would post here in case someone has a better approach.

@ivanjonas
Copy link

Since using babel-plugin-module-resolver does not actually correctly transform the generated *.d.ts, I was able to create a workaround (customizing tsdx.config.js) to use ts-transform-paths and ttypescript

This was a great solution. Thank you @ankit. The key for me was to discover that the code above is only half of the equation. The other half is that tsconfig.json needs a compilerOptions.plugins entry for @zerollup/ts-transform-paths. Docs reference.

joeflateau pushed a commit to joeflateau/tsdx that referenced this issue Nov 12, 2021
since jest v27, the default test env has been changed to 'node'.
this commit configured it to 'jsdom' in react templates.
refs: https://jestjs.io/blog/2021/05/25/jest-27#flipping-defaults
@bryanltobing

This comment was marked as spam.

@agilgur5
Copy link
Collaborator

agilgur5 commented Apr 10, 2022

Thought I'd note here that while I didn't get around to finishing a Babel plugin for this, I did find two Babel plugins that I thought I might forward folks to: babel-plugin-tsconfig-paths as well as babel-plugin-tsconfig-paths-module-resolver. The latter is similar to the method I was using with babel-plugin-module-resolver under-the-hood, so the author beat me to the chase there 🙂

That is useful for consistent usage with Babel (e.g. @babel/preset-typescript, babel-jest, etc), but @ankit pointed out a major flaw with type declarations above that I hadn't realized before, so perhaps @zerollup/ts-transform-paths and ttypescript are the only way of doing this for now when it comes to building libraries with type declarations 😕

Unfortunately, as I've mentioned before, this usage of paths is beyond TS's intent and plugins are not an official TS feature, hence the hackiness (on top of Rollup and TSDX's complicated configs). My intent with a Babel plugin was to hopefully simplify a bunch of that with a more official route for lots of users (notably, I don't use this feature myself), but alas type declarations seem beyond that scope for now -- at least the above Babel plugins will work for apps and probably every use-case outside of type declarations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New feature or request scope: upstream Issue in upstream dependency solution: workaround available There is a workaround available for this issue topic: TS Paths Aliases Related to using aliases with TypeScript paths
Projects
None yet
Development

No branches or pull requests