Full support for monorepositories

[bashmish]

Description

I want to use open-wc testing configs in this monorepository using lerna and using a single test page context (basically run tests once for the entire repo instead of running it per package).

Issues

Some challenges I'm facing right now:

1. wallabyjs config does not work out of the box and requires a preprocessor (I introduced it here) to prevent double loading of the same ES modules
2. when switching to new testing config which also uses native ES modules same problem happens to karma that happened to wallabyjs

Example of a module which gets loaded twice:

• import { LionFieldset } from '@lion/fieldset'; (e.g. here)
• import { LionFieldset } from './src/LionFieldset.js'; (e.g. here)

It causes the double registration of the same custom element.

Error: Failed to execute 'define' on 'CustomElementRegistry': this name has already been used with this registry
      at node_modules/@lion/fieldset/lion-fieldset.js:41:16HeadlessChrome 74.0.3729 (Mac OS X 10.14.4) ERROR

So the first path got rewritten from bare module specifier to a relative path and for the LionFieldset we'll have 2 different paths in the same page context:

• import { LionFieldset } from '../../../node_modules/@lion/fieldset/index.js' (which uses './src/LionFieldset.js' unde the hood)
• import { LionFieldset } from './src/LionFieldset.js';

You see now that the symlinks which lerna creates are not handled correctly since a browser while loading an ES module has no idea that these different paths point at the same location on the file system. That makes sense since a browser just makes a request to a web server and caches modules by final URLs, not by the paths resolved on the file system level.

Proposed solution

Would be nice to have a reusable preprocessor exposed in open-wc that automatically fixes double loading of all monorepository packages.

The preprocessor I created here basically rewrites all bare module specifiers for lerna packages to relatives paths, so that all paths to those packages become relative. E.g. the above problem with LionFieldset will be gone because both paths will resolve to the same thing relatively without involving symlinks in node_modules:

• import { LionFieldset } from '../../fieldset'; (e.g. here)
• import { LionFieldset } from './src/LionFieldset.js'; (e.g. here)

And these paths from a browser point of view will point at the same URL.

[LarsDenBakker]

I also ran into this issue when adding the new test setup to open-wc which is also a monorepo. We're telling the resolve algorithm to follow symlinks, and so it ends up pointing to the correct file.

What I've noticed with monorepos is that sometimes a symlink isn't created for some projects. Can you check that there are proper symlinks in the repository?

[bashmish]

@LarsDenBakker yes, there is a proper symlink and this is why the path '../../../node_modules/@lion/fieldset/index.js' is resolved correctly since node_modules/@lion/fieldset is linked to packages/fieldset. But like I said the browser is not aware that this is a symlink and when the path './src/LionFieldset.js' will be loaded in the fieldset package tests themselves, there will be a double registration exception.

Which resolve mechanism in open-wc are you referring to? Is it using ES modules natively or it's still a webpack driven test files build?

[daKmoR]

I'm just thinking out load but could this mean we don't even need babel anymore at all?

that would be a major speed improvement as I have the feeling that ~80% of the startup time is just babel...

and later we could replace the preprocessor with native import maps (in ~6 weeks when they hit in chrome 75)

[bashmish]

I don't think this is wise strategy to get rid of babel in the long run. You will always need it to transpile emerging standards. I think the strategy should be to remove the ones that landed natively in the browser and only add the ones that are Stage 3 and not yet natively supported.

[LarsDenBakker]

I don't see why we would not need babel. Until we have import maps, we need something to transform the modules. That preprocessor doesn't do a node resolve algorithm. Though we could create regexp to find all imports and rewrite them in a way that's much faster than full babel AST generation. Dynamic imports would be a bit problematic there.

open-wc uses karma with es modules to run the tests. We use a babel plugin for resolving modules, it rewrites the paths of bare imports to a path to the actual file on the file system. It follows symlinks, so that when it resolves a module to /node_modules/@lion/fieldset/index.js it will then follow that symlink and end up at /packages/fieldset/index.js.

That's the theory at least, and it worked like that for open-wc repository. But it's possible there is something different in your setup.

I'll see if I have time to look into this this weekend.

[bashmish]

I checked the karma-esm package (middleware, preprocessor) and didn't find where the symlinks were handled. Can you give some hint where to look at so that I could fix it myself?

[LarsDenBakker]

This is done at: https://github.com/open-wc/open-wc/blob/master/packages/karma-esm/src/forked-babel-plugin-bare-import-rewrite.js#L71

[LarsDenBakker]

@bashmish did you have any luck?

[bashmish]

@LarsDenBakker no, I didn't, but looks like you had. It works now and we can run tests natively with lighting speed at start time which is awesome!

[LarsDenBakker]

Yeah the solution ended up being relatively straightforward. :)

[bashmish]

I upgraded to the fixed version here ing-bank/lion#46 and it works well with HeadlessChrome, but it does not work with BrowserStack config with real Chrome :(

Do you have any idea from the top of your head how this BS karma config can break default working config and cause 55 failed test with the very same error Failed to execute 'define' on 'CustomElementRegistry': this name has already been used with this registry?

[bashmish]

Sorry, my bad. yarn test:bs used --legacy argument, so that's why it still used webpack and the configuration was not right. Now works well!