An alternative to relative imports for local source code

[epixa]

The problem

Today, we import local source code with many different path formats. Client-side code can use webpack aliases, TypeScript can import from kibana, ui, or plugins (technically on either server or client), server-side JavaScript can only do relative imports.

This is a burden for us, so we agreed to fix it. Since relative imports are usable anywhere (in theory), we decided all imports should be done through relative imports. This solves the consistency problem, but it introduces others.

Relative imports across module/package/plugin boundaries inherently make those modules less portable if the system around them changes. A great example of this is the recent move of the legacy plugins, which involved updating hundreds of import paths despite the intention of those import statements remaining unchanged.

Relative imports create ambiguity with similarly named files and directories. e.g. At a glance, does import from '../../../../../../tests' import from the root level tests directory or from the tests directory inside your plugin? The deeper your code is nested, the harder that question is to answer. It's not purely visual as well, as you can't really use find+replace to identify this either.

Relative imports make many developers unhappy. There has been strong push back on relative imports for sharing code across plugins, especially from folks that do most of their development in the browser where they're used to convenient webpack aliases.

Finally, the assumption we made when adopting a relative imports strategy that they are available in all contexts is not accurate. Despite them seeming like the standard import mechanism, developers have run into issues in various places (e.g. mocks in tests) when trying to do relative imports.

Proposed solution

Code should be imported from outside the current package/system/plugin using imports from Kibana root.

Within a package/system/plugin, it's appropriate to use relative imports, though absolute imports are still acceptable when the relative import is still crazy. Most developers will do this anyway given the option.

So for example, any of these options would be OK from within a plugin called "foo":

/*
src
  plugins
    foo
      bar
        whatever.js
      test_utils
        foomocks.js
test_utils
  globalmocks.js
*/

// src/plugins/foo/bar/whatever.js

// OK
import from 'test_utils/globalmocks';
import from 'src/plugins/foo/test_utils/foomocks';
import from '../test_utils/foomocks';

// NOT OK
import from '../../../test_utils/globalmocks';

I have a proof of concept for supporting this import behavior in #40066. More work is required, like updating the webpack configuration for client-side code and getting CI passing, but overall I think this is very doable.

[elasticmachine]

Pinging @elastic/kibana-platform

[epixa]

cc @elastic/kibana-operations

[epixa]

I should mention, these sorts of imports do not really solve the portability problem, they just make the portability problem significantly easier to deal with since you can universally use find+replace for access to code within plugins

[jasonrhodes]

I'm generally +1 on anything that removes ../../ from our code. One thing to mention, the more surface area you take up on the import from 'something_magic' front, the more likely it becomes you accidentally run up against a real import named something_magic ... for that reason at my last job we picked a single root word and made all imports start with that, so if you saw an import that started with src you knew it came from our root.

We could use import from 'kibana/whatever/path' or 'kibana_root/whatever/path' or even just 'root/whatever/path' but I like the idea of having a single indication that an import is coming from a non-standard module resolution location. $0.02.

[epixa]

@jasonrhodes Unfortunately, we're limited by the implementation here. For client-side code and for typescript, we can specify any common root we want, but for server-side javascript, all we can do is specify a root path that is literally part of the file path. In other words, to support that, we'd need to move all of our source code under a common top level directory within the project. kibana/whatever/path would need to literally be $KIBANA_ROOT/kibana/whatever/path on the file system.

I think in practice we'll be OK here for Kibana's case, but it's a fair point.

Edit: The vast majority of imports in this case would be into one of three directories, src/, x-pack/, and far less frequently test/. So we're not in too bad shape.

[spalger]

I really like your suggestion @jasonrhodes.

I really wish we could solve this by just going full monorepo and making everything a package, but I realize how much work that is.

What if instead of webpack aliases and $NODE_PATH we used a babel plugin to rewrite the import paths. Then we could:

• stick with the current convention that @kbn/ packages are imported from the repo and not the registry
• limit the root-level directories that support importing, @kbn/src would work for instance but importing from the packages directory wouldn't, they would still need to be imported with @kbn/{name}
• only change the import strategies for our code, no conflicts with transient dependencies named docs or target or something
• expose this to plugins without confusing them with imports to src which don't map to their source directory
• symlink the node_modules/@kbn directories so that IDE tools that rely on node_module resolution will continue to work
• potentially unify the strategy for the server and the client, potentially rewriting ui/public imports as @kbn/ui/public or something.

I've also created a POC for this behavior here #40451

[epixa]

That’s a cool idea if we can get it to behave consistently for js/ts, server/client, and tests. I’ll check out your POC next week

[timroes]

Thanks for having that discussion. I would in general really appreciate having shorter path for the relative imports. My two cents:

I would create some @whatever synonym for the path, instead of a plain kibana/ or src/ prefix, since I like that you see that it's meant to be something special. Not sure if we could even use another non @ char (since that can still reference legal npm organizations).

Given the actual import paths, I would suggest using a different prefix than @kbn. I think using the same prefix that we use for packages might create more confusion again (besides potentially creating more issues in path resolutions in some tools, like Jest). Because @kbn/some-package would import from a package, but just the magic name @kbn/src (and potentially other ones later, as Spencer suggested) would strangely import from a completely different root, purely by the name of that second folder in the path. I see that this could potentially create us more issues and confusion later on then just using a completely different prefix, e.g. @kibana.

Given the concrete entry points I would suggest having thing that are really commonly imported from as a prefix, and not only the src level itself, since if the path might be very deep you still end up with horrible long paths. Still I would consider very carefully about adding new paths here. My suggestions would be adding something like:

• @kibana/ -> src/
• @plugins/ -> src/plugins/ (maybe even for the transition time, also src/legacy/core_plugins)
• @kbn/ -> packages/ (as it is today)

[spalger]

I like that idea @timroes, especially the part about finding a prefix that isn't a usable npm org prefix. In my POC I was able to get the following to work by editing the config.

• @src/
• $src/
• #src/
• *src/
• ()src/
• !src/

Choosing how to define the prefix is now going to be the hardest part of this 😅

[timroes]

I would vote for either $kibana (still not a huge fan of naming it src :D) or @kibana (despite the potential overlap with NPM orgas), or maybe #kibana (as the least favorite option). The others look imho simply too much 😱 to use them

[spalger]

I'm also not a fan of importing $src directly, but I think we were probably going that route because we wanted to have a root that people to go to in the source and look up files... What if we just exported $core and $legacy explicitly instead of everything in the src directory?

[timroes]

@spalger I would totally support that approach. I think we should only export places we know there is a high chance users commonly use them. So I think $core and $legacy, $legacy (and maybe $plugins separately?) is better than just offering $src.

[spalger]

Oh, something I forgot to test is how this works in filesystems when creating links to the imports from the node_modules directory. $ should work, but I don't think we'll be able to use some characters like : because of this, in case someone is trying to get creative :)

[jasonrhodes]

As long as it's clear and easy to add new directories to the white list, locking it down specifically is probably fine. In practice, I find that usually leads to people deciding to put something in the core or legacy directories even though the new thing doesn't really belong there, just to get the easy sharing. So having a single top-level alias that lets you import from anywhere below it seems okay to me, cause engineers gonna engineer anyway. :D

[epixa]

I'm OK with a solution that doesn't rely on NODE_PATH, but I don't support the level of customization that's been proposed since last week.

To start, preventing people from importing certain things is not part of the problem I described here. Today people can import from root in typescript and from relative paths in javascript/typescript, and we don't have a widespread problem of people importing code into the distribution from outside src/x-pack.

If restricting imports was part of the problem I was trying to solve here, I'd strongly encourage the use of linting instead of customizations to our build tooling. We're already using linting rules to help enforce some of the emerging best practices for importing with success, and having all such rules handled via linting allows for more consistent development. Plus, linting rules can be disabled for exceptional cases we didn't consider and don't necessarily want to change our build system to support.

My preference is not to use a prefix. Paths are a universal language, and we've embraced that with the new platform's goal of having the file system reflect the architecture of the project. Importing from project root is already a feature used in the wild with NODE_PATH as well as the basic configuration for typescript projects. We have hundreds of these imports in Kibana already, and unlike the magic paths built into our webpack config, I've never heard nor seen any developer questioning which files those imports reference. I don't even know if we intended to introduce this behavior for typescript, I suspect it was an accidental side effect of introducing typescript and people embraced it because it feels pretty natural.

I'm strongly opposed to customized import behaviors that proxy to a nested place in the repository. We've been working for years to try to make how Kibana functions as a piece of software less unique. It's been a monumental expense of developer time, company focus, community pains as we break things, etc.

[spalger]

Alright, I miss understood "Unfortunately, we're limited by the implementation here." as "We'd do it if we could"...

Do people generally think that importing from src/... is going to be clear? What if we renamed the src directory to kibana so that imports were generally based from kibana/core/... or kibana/legacy/ui/...?

There are so many src directories, I really don't find a bunch of import paths starting with src to be very clear. Ultimately though, I can't argue with the fact that NODE_PATH is commonly used in the wild. I've never used it in the wild but my experience outside of Kibana is very limited.

[epixa]

You didn't misunderstand me, I just formed a more concrete opinion of it once you showed it was possible.

Do people generally think that importing from src/... is going to be clear?

I can't say for certain, but people have already done it 200-something times since in the last few months since it became possible when there wasn't any history of it in the repo for years before. I think people pick up pretty quickly on whatever directory scheme we have, and since src has been there for so long, it's especially obvious to existing devs.

What if we renamed the src directory to kibana so that imports were generally based from kibana/core/... or kibana/legacy/ui/...?

I think src is fine personally, but I don't have strong opinions that the directory shouldn't have a different name. The actual effort to rename it might be hell, for what it's worth. I think kibana isn't the right name since it's missing a huge amount of source code that is part of Kibana (x-pack), but I'm not against the premise here.

[timroes]

I must say I agree with Court on that part about not using the pathes for import prevention. We should rather use linting for that.

Given the prefixes I share a different opinion. Despite paths are universal, I would want to argue that also using prefixes in web development has become quiet universal. You'll find quiet some "standard presets", e.g. like the famous electron-webpack build system, that uses aliases to improve developer experience. And tools that try to prevent that improve of developer experience and requiring full paths, usually get quiet some pushback on that opinion, e.g. see the create react app PR, where the denial of the very similar request got 51 👎 and 0 👍 reactions (at the time of writing).

Thus I think many developers are actually used to having prefixes as part of their developer experience to improve long relative paths, so I wouldn't agree with having prefixes makes "Kibana as a piece of software more unique".

I agree with Spencer regarding the naming of src as an entry point, since there are really a lot of src points, and I think we should make this name more unique (which imho would happen by giving it a unique char lead prefix, like @kibana or @xpack if that's needed).

[jasonrhodes]

FWIW I was imagining that whatever the single magic token is (src, @kibana, whatever), that it would resolve to the actual kibana root. So you'd import from @kibana/src/core_plugins/whatever or from @kibana/x-pack/plugins/whatever. Then as a couple of you have said, linting can help with restricting import paths that aren't valid etc. Just a useful little "hey this isn't a real node package being imported, it's our special kibana NODE_PATH thing".

[streamich]

Can we have double @@ to denote "root" folder? For example:

• @@/src -> /src
• @@/src/plugins -> /src/plugins
• @@/x-pack/plugins -> /x-pack/plugins
• etc.

[epixa]

Can we have double @@ to denote "root" folder?

I'm not sure what problem this aims to address, can you clarify?

Thus I think many developers are actually used to having prefixes as part of their developer experience to improve long relative paths, so I wouldn't agree with having prefixes makes "Kibana as a piece of software more unique".

It's not the existence of aliased imports that make Kibana unique, it's the aliases themselves. Even the concept of "core" is unique to Kibana, requiring an understanding of "what is 'core' in the context of the Kibana project" to understand where the import statement maps to in the code base.

And our plugin imports make this even more confusing since plugins can exist in multiple locations, which either means we have some magic plugins import that maps to multiple potential locations on disk and somehow accounts for the possibility of conflicts or we have some separate import prefix based on the type of plugin, which we already have in our directory structure anyway.

I agree with Spencer regarding the naming of src as an entry point, since there are really a lot of src points

Can you elaborate on this? I don't think src is especially ambiguous in our case. Everyone working on the Kibana project has encountered our top level src directory every time they open the repo in github, in their editor, etc. Anyone not familiar with the Kibana repo is going to discover our src directory the first time they look at the repo.

I think src is even less ambiguous in practice since you will always be importing from a directory under it.

import from 'src/core/public';
import from 'src/plugins/foo/public';
import from 'src/test_utils';

I understand the concern in principle about the generic-ness of src, but in practice this seems like a non-issue. Or at least a trivial enough issue that it doesn't warrant the effort of renaming the directory, for example.

[lukeelmers]

I think src is even less ambiguous in practice since you will always be importing from a directory under it.

This is what I was thinking. I don't intend to trivialize any of the concerns raised here, but I guess I don't see how someone reading an import that starts with src/core, src/plugins, x-pack/plugins, etc, would not be able to immediately understand where it is pointing. IMO using paths requires zero explanation to devs jumping into the project for the first time.

But perhaps I am oversimplifying; I could get on board with using a special token if others find that more intuitive, I'm just reticent about seeing more magic aliases added to Kibana 😉

[mshustov]

Will we abandon webpack alias and typescript path approaches, if we switch to any solutions from proposed above? Usage any of non-standard mechanisms adds maintenance costs since we can no longer just start using code analysis tools.
we already have this problem for linter and a dependency resolution tool.
How we are going to integrate our custom solution in 3rd party tools?

[epixa]

@restrry I'm not sure how the babel approach would work in that case, so I'll defer to @spalger on that.

If we went the NODE_PATH approach, then we'd have three mechanisms that dealt with parsing the import paths:

• TypeScript base path
• Webpack for client-side JS
• NODE_PATH for server-side JS

As far as I know, that's about as "standard" of a set of mechanisms available to us.

[spalger]

If we went the NODE_PATH approach, then we'd have three mechanisms that dealt with parsing the import paths:

TypeScript base path
Webpack for client-side JS
NODE_PATH for server-side JS

As a person who will be responsible for maintaining this, I would certainly prefer that it was implemented with a babel plugin rather than webpack aliases/NODE_PATH. I mostly prefer that because I know the scope of babel plugins, they aren't applied to node_modules, they aren't automatically inherited by child processes, and we can further customize their behavior if we need that down the road.

Configuration options for webpack aliases and NODE_PATH haven't changed in years and are very simple, but also applies way more widely than I think is necessary and I'd suspect have downsides we're not seeing.

Usage any of non-standard mechanisms adds maintenance costs since we can no longer just start using code analysis tools.

I agree, but I expect this to be a drawback of adding support for src/*, not a specific approach. That said, every tool we use requires configuration of some sort to work correctly so I'm not concerned.

I don't intend to trivialize any of the concerns raised here, but I guess I don't see how someone reading an import that starts with src/core, src/plugins, x-pack/plugins, etc, would not be able to immediately understand where it is pointing.

My concern is mostly for third-party devs. src/core would surprise me if I was working in a newly generated plugin, but people will just have to learn.

[epixa]

@spalger Works for me

[epixa]

@spalger Thanks for clarifying.

@brianseeders I'll give that a go, thanks! I do like leaning on setup_node_env as much as we can.

[epixa]

Blast, I was doing a bit more research on NODE_PATH, and I think we may want to avoid it after all. While it hasn't made its way to the node docs as far as I can tell, node team members have indicated in GitHub that NODE_PATH is "deprecated", incompatible with the new ES module flag, and will not likely have a direct replacement. In fact, the ESM proposal explicitly rejects all mechanisms to change path resolution behavior. This means that relying on it heavily in Kibana could one day hold us back from upgrading node, and that is not really acceptable.

So to recap:

• NODE_PATH will not be available to us in the future
• The babel plugin approach adds a lot of complexity and the third-party module we were going to rely on is abandonware with known bugs on newer node versions

I think there are two remaining options that could work for us:

• Symlink Kibana root into node_modules. This is the only documented approach from the node team itself to dealing with this sort of behavior, though they discourage this in favor of actually building your software as node_modules, which we're not doing. Given the generic-ness of our directory names, we probably would want to do this as a prefix similar to the recommendation in An alternative to relative imports for local source code #40446 (comment).
• Ignore JavaScript and focus on getting TypeScript working consistently. We are moving the entire Kibana project to TypeScript, and over the last year or so the repo already has more TS than JS. Given that this functionality is baked into TypeScript by default, maybe the right solution here is to close the gaps that are making its usage in TS inconsistent (e.g. jest mocking).

Thoughts?

[spalger]

We could definitely write the Babel plugin ourselves. Seems like relatively simple babel plugin to write.

But I'm also fine with only supporting this type of resolution in TypeScript, except that I think since we use babel to transpile our TS we're going to need to support this in babel anyway right?

[epixa]

We could definitely write the Babel plugin ourselves. Seems like relatively simple babel plugin to write.

I'm not necessarily sure I agree with this, but I haven't dug into it enough to say for sure. But the babel plugin we were going to use was 1700+ lines of JS, and it is broken in ways that require uses to wipe out their babel cache under certain conditions. We'd likely start stepping into the weeds of a definition of "simple" if we go down this route at the least.

But I'm also fine with only supporting this type of resolution in TypeScript, except that I think since we use babel to transpile our TS we're going to need to support this in babel anyway right?

TypeScript compiles to a relative path, so AFAIK there wouldn't be any sort of requirements on babel for that approach.

[spalger]

Yeah, cache invalidation when configuration changes would certainly be the tricky bit here, but since we don't want multiple rules and we will plan to "never" change the configuration, import 'src/x/y/z' will always and forever just need a simple transformation to import '{relativeDots}/src/x/y/z', without having to worry about cache invalidation 😄

TypeScript compiles to a relative path, so AFAIK there wouldn't be any sort of requirements on babel for that approach.

Are you implying that typescript is currently compiling to a relative path? Typescript already works?

[joshdover]

Do we know if the TypeScript compiler approach also works for Jest test files? I know Jest does it's own module resolution for its mocking purposes, but I'm not sure how this interacts with the tsconfig.

[epixa]

Do we know if the TypeScript compiler approach also works for Jest test files?

As far as I know, there is no automatic way to make this work. Regardless of which approach we take, jest will need to be configured with the proper modulePaths.

Yeah, cache invalidation when configuration changes would certainly be the tricky bit here

Config changes weren't the most serious issue - moving files like src/foo.ts -> src/foo/index.ts would break due to cache as well.

Are you implying that typescript is currently compiling to a relative path? Typescript already works?

I was implying that, but you prompted me to go check on this, and it turns out I'm wrong. I had seen TS project-level -> relative path compilation happen, but it must have been during my effort with the babel plugin. I just tested to see if project-level imports worked for distributed code, and it does not. It looks like all the existing imports using project-level resolution are for types.

So this doesn't work anywhere today, but it can through symlinks or babel plugin (mostly from scratch). On principle I like the idea of symlinks more because they're the "blessed" approach, but it could also cause a cascade of issues that I haven't thought of since we're doing things like bundling up a vendor dll for node modules.

Perhaps we just need to throw together working examples of both to compare.

[lizozom]

@epixa what would you say is the status on this ATM? What style of paths should developers migrating to NP use?

[epixa]

@lizozom The latest comments on here are an accurate reflection of the status. I'm planning to put together a POC of the symlink approach to see if that shakes loose any horrible surprises, but I haven't started on it. I'm open to anyone else giving that a go as well, but I'm not sure who has time.

At this point, relative paths are still the only option that works almost everywhere.

[spalger]

I've got a POC of a babel plugin that replaces imports at build time with relative imports, and it's really simple:

const Path = require('path');
const t = require('babel-types');

module.exports = () => ({
  name: '@kbn/babel-preset/transform/rewrite_absolute_imports',
  visitor: {
    ImportDeclaration(path) {
      const source = path.get('source');
      const importPath = t.isStringLiteral(source.node) && source.node.value;

      if (!importPath || !(importPath.startsWith('src/') || importPath.startsWith('x-pack/'))) {
        return;
      }

      const absPath = Path.resolve(path.hub.file.opts.root, path.hub.file.opts.sourceFileName);
      const targetPath = Path.resolve(path.hub.file.opts.root, source.node.value);
      source.replaceWith(t.stringLiteral(Path.relative(Path.dirname(absPath), targetPath)));
    }
  }
});

This only applies to src/* and x-pack/* imports, which isn't necessarily the direction we need to go, but I'm a fan of this approach and going to try and put up a PR of it soon (along with the related eslint changes necessary to enforce use of src/* vs ../../../../src etc.)

[epixa]

Same-name variables differentiated by capitalization. You animal.

[spalger]

Same-name variables differentiated by capitalization. You animal.

I've sort of given up on Url and Path, I use them all the time and they constantly conflict with local variables when lower cased. Even worse, their named exposed constantly conflict too... Naming is hard...

[pgayvallet]

So, after discussion with @epixa, I opened #52995, which is based on @spalger work and adds the (very few) missing bits.

PR focuses on actually implementing the absolute path imports handling. I would like to keep the eslint rules to enforce their usage and the associated big-diff auto-fixes in a separate PR if that's alright with everyone.

There are still a few questions open in the PR on some specific import rules that changed due to the fact that eslint now properly recognize src/x-pack as internal imports instead of modules in some part of the code.

Comments/Reviews are welcome.

[fbaligand]

Hi,

My 2 cents on the topic, as a community plugin developer:

Currently, some JS/TS Kibana-core components are not easy to import in a Kibana plugin.
For example, for now, to get "setup" dependency, I have to do this import:
import { setup as visualizations } from '../../../src/legacy/core_plugins/visualizations/public/np_ready/public/legacy';

This is not very elegant, and is strongly based on physical plugin installation directory: $KIBANA_HOME/plugins/my-plugin

So it is not portable.