untool / untool Goto Github PK

Add a CLI option to the build and start commands to build/start specific targets (node, develop, graphql-mock-server (as defined by Hops), service-worker (as defined by Hops)).

This would allow us to only start the GraphQL mock server in a Hops app, in order to have the server start without having to wait for the node and browser build to succeed when only the mock server is required.

We've figured that untool should do less and leave the more specific implementations to other projects, such as https://github.com/xing/hops.

In this ticket we'll try to collect concerns that untool could care less about:

• express compress middleware
• static rendering?
• postcss / css config
• isESNext/isExternal ?
• react

This issue was opened in Hops repo, this is its follow-up.

untool/react iterates over a moduleFileMap object which contains a few invalid keys ("" and null) when:

• Webpack's ModuleConcatenationPlugin is introduced in production mode by untool/webpack.
• User dynamically import re-exported modules

It seems to be a known webpack semi-issue, in relation to which Webpack's maintainer suggested to use an alternative way to identify generated modules.

Avoiding to spread non-iterables entities here seems to fix the issue (but I couldn't guarantee about possible side-effects 😃):

  const moduleFiles = modules.reduce(
    (result, module) => {
      const moduleFile = moduleFileMap[module] || [];
      return[...result, ...moduleFile]; 
    },
    []
  );

While initially running my app compiles just fine, however on editing any of the app's files, the watcher crashes:

untest:error TypeError [ERR_UNKNOWN_SIGNAL]: Unknown signal: SIGUSR2

at process.kill (internal/process/per_thread.js:193:15)

at Watching.compiler.watch [as handler] (.\node_modules\@untool\webpack\lib\middlewares\render.js:50:19)

at compiler.hooks.done.callAsync (.\node_modules\webpack\lib\Watching.js:98:9)

at AsyncSeriesHook.eval [as callAsync] (eval at create (.\node_modules\tapable\lib\HookCodeFactory.js:32:10), <anonymous>:15:1)

at Watching._done (.\node_modules\webpack\lib\Watching.js:97:28)

at compiler.emitRecords.err (.\node_modules\webpack\lib\Watching.js:72:19)

at Compiler.emitRecords (.\node_modules\webpack\lib\Compiler.js:449:39)

at compiler.emitAssets.err (.\node_modules\webpack\lib\Watching.js:53:20)

at hooks.afterEmit.callAsync.err (.\node_modules\webpack\lib\Compiler.js:435:14)

at AsyncSeriesHook.eval [as callAsync] (eval at create (.\node_modules\tapable\lib\HookCodeFactory.js:32:10), <anonymous>:15:1)

The signal SIGUSR2 is not defined in Windows environments. I'm not sure if there is a way to kill the process on windows by another call. Seems like we need to fallback to webpack/hot/poll on windows, as suggested here: sysgears/apollo-universal-starter-kit#14

I'm getting a couple of errors, when installing a new project, because @babel/preset-react has a peer dependency to @babel/core.

I don't think the downstream project should have to install @babel/core directly, because they don't use it in their applications.
Therefore I suppose that @untool/react should bring its own @babel/core.

Skimming through the code, it looks like @untool/webpack also has a direct dependency to @babel/core, same goes for jest-preset-hops.
On the topic of Hops: hops-react also adds babel plugins which have peer dependencies to @babel/core, therefore, both, @untool/react and hops-react should bring their own @babel/core I think.

 ~/workspace/untool/tests/fixtures/untest (master) $ ../../../node_modules/.bin/un -h
Usage: un <command> [options]

Commands:
  un serve    Serve untest
  un build    Build untest
  un develop  Serve untest in watch mode
  un start    Build and serve untest

Options:
  --help, -h   Show help                                               [boolean]
  --quiet, -q  Silence log output                                      [boolean]
 ~/workspace/untool/tests/fixtures/untest (master) $ ../../../node_modules/.bin/un --help

Same for Hops CLI and also for help pages of sub commands, as in un serve -h / un serve --help.

This issue contains a list of Renovate updates and their statuses.

Rate Limited

These updates are currently rate limited. Click on a checkbox below to force their creation now.

• fix: update dependency ajv to v7 (master)
• fix: update dependency cosmiconfig to v7 (master)
• fix: update dependency enhanced-resolve to v5 (master)
• fix: update dependency find-up to v5 (master)
• fix: update dependency helmet to v4 (master)
• fix: update dependency is-plain-obj to v3 (master)
• fix: update dependency serialize-error to v8 (master)
• fix: update dependency serialize-javascript to v5 (master)
• fix: update dependency supports-color to v8 (master)
• fix: update dependency terser-webpack-plugin to v5 (master)
• fix: update dependency webpack to v5 (master)
• fix: update dependency webpack-dev-middleware to v4 (master)
• fix: update dependency webpack-sources to v2 (master)
• fix: update dependency yargs to v16 (master)
• chore: lock file maintenance (master)
• chore(workflows): update actions/checkout action to v2 (v2.x)
• chore: update commitlint monorepo to v11 (v2.x) (major) (@commitlint/cli, @commitlint/config-conventional, @commitlint/travis-cli)
• chore: update dependency execa to v5 (v2.x)
• chore: update dependency puppeteer to v5 (v2.x)
• chore: update dependency supertest to v6 (v2.x)
• chore: update react monorepo to v17 (v2.x) (major) (react, react-dom)
• fix: update dependency ajv to v7 (v2.x)
• fix: update dependency cosmiconfig to v7 (v2.x)
• fix: update dependency enhanced-resolve to v5 (v2.x)
• fix: update dependency find-up to v5 (v2.x)
• fix: update dependency helmet to v4 (v2.x)
• fix: update dependency is-plain-obj to v3 (v2.x)
• fix: update dependency serialize-error to v8 (v2.x)
• fix: update dependency serialize-javascript to v5 (v2.x)
• fix: update dependency supports-color to v8 (v2.x)
• fix: update dependency terser-webpack-plugin to v5 (v2.x)
• fix: update dependency webpack to v5 (v2.x)
• fix: update dependency webpack-dev-middleware to v4 (v2.x)
• fix: update dependency webpack-sources to v2 (v2.x)
• fix: update dependency yargs to v16 (v2.x)
• chore: lock file maintenance (v1.x)

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore: update commitlint monorepo to v11 (master) (major) (@commitlint/cli, @commitlint/config-conventional, @commitlint/travis-cli)
• chore: update dependency canarist to v2 (master)
• chore: update dependency eslint to v7 (master)
• chore: update dependency execa to v5 (master)
• chore: update dependency puppeteer to v5 (master)
• chore: update dependency supertest to v6 (master)
• chore: update dependency uuid to v8 (master)
• chore: update react monorepo to v17 (master) (major) (react, react-dom)
• fix: update dependency duplitect to v3 (master)
• fix: update dependency escape-string-regexp to v4 (master)
• fix: update dependency mixinable to v5 (master)
• fix: update dependency pretty-ms to v7 (master)
• chore: update dependency canarist to v2 (v2.x)
• chore: update dependency eslint to v7 (v2.x)
• chore: update dependency uuid to v8 (v2.x)
• fix: update dependency duplitect to v3 (v2.x)
• fix: update dependency escape-string-regexp to v4 (v2.x)
• fix: update dependency mixinable to v5 (v2.x)
• fix: update dependency pretty-ms to v7 (v2.x)
• chore: lock file maintenance (v2.x)
• Check this option to rebase all the above open PRs at once

Ignored or Blocked

These are blocked by an existing closed PR and will not be recreated unless you click a checkbox below.

• chore: update dependency ava to v3 (master)
• chore: update dependency cross-env to v7 (master)
• chore: update dependency ava to v3 (v2.x)
• chore: update dependency cross-env to v7 (v2.x)

• Check this box to trigger a request for Renovate to run again on this repository

There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.

Error type: Cannot find preset's package (@untool/renovate-config:base)

Adding the package Storybook Addon Knobs (that is using a preset file with functions) breaks the build.

To reproduce:

• Go to tests/fixtures/untest
• Add the package: yarn add @storybook/addon-knobs --dev
• yarn start in the root

untest:info running 'start' in 'development' mode
untest:warning Invalid configuration value(s) detected:
- config['managerEntries'] should not be a function
- config['config'] should not be a function
untest:info listening at http://localhost:8080/
@untool/react deprecated [DEP001] Using a string as loader for `importComponent` is deprecated and will be removed in a future major release (https://github.com/untool/untool/blob/master/DEPRECATIONS.md). <anonymous>:null:null
@untool/react deprecated [DEP001] Using a string as loader for `importComponent` is deprecated and will be removed in a future major release (https://github.com/untool/untool/blob/master/DEPRECATIONS.md). <anonymous>:null:null
untest:info bundling 'node' failed after 5.9s
untest:error BuildError: ./node_modules/@untool/core/lib/config.js
Module not found: Error: Can't resolve '../register' in './node_modules/@untool/core/lib'
untest:error BuildError: ./node_modules/@untool/core/lib/config.js
Module not found: Error: Can't resolve './addDecorator' in './node_modules/@untool/core/lib'

We've discovered that there is a need to specify which config values should be sent to the client / or which values contain secrets and should therefore only be exposed to the server-side implementation.

There are a few solutions for white-listing values for the front-end that come to mind:

1. indicate through a naming scheme of the key, that this value should be secret (eg. _foo)
2. indicate through special characters in the value (eg. double brackets [[FOO]]) that this value should be secret
3. use an array with white-listed keys, that will be merged automatically (to allow each preset to add their own values) to indicate which values should be forwarded to the client
4. use an object that holds a mapping of keys to boolean values that should be white-listed (in contrast to the previous suggestion, this would allow users to override values set by presets)
5. allow an object as value that contains two fields (value and expose or similar) to configure that specific field

{
  "name": "untest",
  "license": "MIT",
  "untool": {
    "_variant_1": "[SECRET]",
    "variant_2": "[[SECRET]]",
    "variant_3": "[NO_SECRET]",
    "whitelist_array": [
      "variant_3"
    ],
    "variant_4": "[NO_SECRET]",
    "whitelist_object": {
      "variant_4": true
    },
    "variant_5": {
      "value": "[NO_SECRET]",
      "expose": true
    }
  }
}

Reason

Right now the snapshot test does not run on windows, because the path matching in the normalize helper fails to match for windows paths.

The details in the normalizer work on all strings matching a base path. This needs to convert backslashes as well and not only remove the common base path.

Problem

I think a few things about this are awkward:

1. Replacing strings by regexp in a serialized json object could match other things and not only paths.
2. Converting all backslashes for 'kind-of'-paths is not that easy and would increase complexity a lot
3. This is 'patching' the test results before snapshoting them. This means to create code for just the tests to have them more stable which is contraproductive and could lead to fixing the testcode instead of bugs.
4. Snapshots per se are more useful to detect if something has changed, but unit tests should be there do define whats expected. Right now the expectation in the untool tests with snapshots is not clear.

Solution

• Rewrite the tests using snapshots into more explicit tests.
• Maybe convert the tests to jest instead of use ava since the expect output is somewhat better

Just a quick thought:

Would it make sense to combine the develop and build configs into one and use conditionals to include / enable / disable certain plugins for each use-case (eg Split Chunks Plugin, HMR Plugin, Uglify Plugin, etc).

Then we wouldn't have to maintain two separate configs which are mostly identical and the separation between Node / Browser would be easier to understand.

Due to jest’s incompatibility with latest node versions, untool’s canarist checks currently fail since we discard hops’ yarn lock file.

The root cause of this issue is being tracked in xing/hops#815.

Looking at our code base, I cannot help but thinking that static rendering support in its current form is somewhat obsolete - and actually rather easy to implement downstream given the APIs/hooks we currently provide.

If we decide not to drop it altogether, is it feasible to at least refactor all its parts into the webpack plugin? I still find it rather strange to have server mocking code floating around @untool/express.

Any opinions, @untool/untool?

I'm currently reviewing / writing readme documentation and I wondered whether the --static option for the development command makes sense at all as it seems that --static is only used when executing build?
https://github.com/untool/untool/blob/master/packages/webpack/mixin.core.js#L192

Just a quick thought I had while working on the pwa preset:
We might want to default basePath (and maybe assetPath too) to / and not allow empty strings.
Then we could always join them as url parts and don't have to think about whether they need to start with or without slash.

TODO: flesh out this idea into a concrete usage example.

Problem:

Right now all middlewares are registered in the same order as the mixins are loaded.
This prevents users from overriding the order without losing the benefit of presets.
It is also not possible for a user to remove a middleware.

Solution (proposal):

Define a registerMiddleware(s) hook (strategy: sequence) that collects all middlewares from all mixins first and in another step read the config for the set of middlewares that is desired. This would also allow a user to ignore/remove a middleware later on.
This requires that all mixins name their middlewares.

Example:

Mixin:

registerMiddlewares() {
  return {
    'my-middleware': (app) => (req, res, next) => {
      ...
    },
  }
}

Config:

{
  middlewares: [
    'my-middleware',
  ],
}

P.S.: Yes, middlewares is a word 😄

I think we have a similar problem.
In our case the config type is hops.

The untool version is 1.10.4.

Sadly on the config are two keys with functions on it and calling item.replace on it fails. So our whole project stops to work :(.

The config passed to the environmentalize function.

{
      rootDir: 'xxxx,
      name: 'yyyy',
      version: '0.2.2',
      managerEntries: [Function: managerEntries],
      config: [Function: config],
      browsers: [ '> 0.5%', 'last 2 versions', 'Firefox ESR', 'not dead' ],
      node: 'current',
      ....
}

https://github.com/untool/untool/blob/v1.10.4/packages/core/lib/utils.js#L75

Miss, Status, and Header are having no effect with react-router v5.1.
It seems like staticContext is no longer being passed into those components.

I'm still debugging and looking for a fix.

as commented here: xing/hops@96ba6df#r31558987

Idea: Make getConfig an overrideable mixin hook, so that we can override it in hops-postcss in order to pass cssLoaderConfig to the loaderConfigs and allLoaderConfigs arguments of the configureBuild hook to allow other mixins that implement configureBuild easier access to the cssLoaderConfig.

Let's discuss if this is a good idea (tm). Off the top of my head this will make hops-postcss order dependent..

With the update to core-js@3 the @babel/preset-env introduced warnings and errors for dependency version mismatches and configuration errors:
babel/babel#7646
See also babel/babel#9716

We should probably set the coreJs option for the @babel/preset-env and print our own warning through the @untool/info package when core-js isn't a top-level dependency in the correct version.

Steps to reproduce:

1. check out the untool/untool master branch and get to the latest state.
2. delete yarn.lock file and re-install all packages.
3. run yarn build and observe the following warning:

untest:info running 'build' in 'development' mode

WARNING: We noticed you're using the `useBuiltIns` option without declaring a core-js version. Currently, we assume version 2.x when no version is passed. Since this default version will likely change in future versions of Babel, we recommend explicitly setting the core-js version you are using via the `corejs` option.

You should also be sure that the version you pass to the `corejs` option matches the version specified in your `package.json`'s `dependencies` section. If it doesn't, you need to run one of the following commands:

  npm install --save core-js@2    npm install --save core-js@3
  yarn add core-js@2              yarn add core-js@3

This issue is a follow-up of xing/hops#819.

@untool/webpack relies on @babel/present-env's useBuiltIns feature which might break when the globally hoisted core-js version doesn't match the one expected by @babel/polyfill.

Solution 1: core-js as peer dependency

A solution might consist of declaring core-js@^2 as @untool/webpack's peer dependency to force the consuming application to keep it as it's own direct dependency

Solution 2: core-js v2 dependency check on build

On compile time@untool/webpack might ensure that the expected version of core-js is available at node_modules root and issue a warning message if not.

Solution 3: custom module resolution

Extend the current custom module resolution to target the polyfill declaration injected by babel and redirect them to the expected core-js library.

As a brownfield developer I want to provide custom markup

In order to migrate my existing app step-by-step to a hops/untool setup I need to add a few additional <script> tags to my server output (I know I could migrate them to be provided by templateData.globals but thats not feasible in my case.)

I would therefor like to overwrite the react template in my server mixin.

I can provide a PR if you'd agree that this would be a valuable feature in this project.
What do you say? @dmbch @ZauberNerd @robin-drexler ?

Currently, server.js is served by the static express middleware, because is written into the dist/ folder.

This should not happen (by default) as this is probably unexpected behavior.

Similar to how we have split the hops-mixin into a separate package, I think it would be helpful to split out the untool Mixin into its own package, because then we could use it as a dependency to the CLI (which right now isn't possible, because then the CLI would have a dependency to the core):
xing/hops#632 (comment)

We've got a few questions about how to prefetch / preload dynamically imported chunks with importComponent.
Right now untool drops these comments in the babel plugin and the server-side rendering does not respect such comments.
We should:

• tweak our babel plugin to keep comments in dynamic imports
• tweak our assets plugin to attach meta information about which chunks to preload/prefetch
• tweak our server-side rendering to add prefetch/preload resource hints

Current status

Right now untool deduces the rootDir from the found (from pwd and up) package.json.
From this process the configuration is loaded using cosmiconfig. This part is not overridable.

Problem

This behaviour is a bit too static.
The first issues arose during unit testing where we currently need to spawn the test in a separate process.
On a broader view this right now prevents integrating untool in a programatic way where the scope is not the current directory. E.g. initalize an untool instance as part of a program where the start-folder should not be the current directory.

Solution

It would be good to set the rootDir from outside and respect this during each and every initialize step (cosmiconfig, webpack, ...)

This came up in a discussion with @skrings about quietly resuming erroneous operation in production mode or failing hard by throwing an exception.

I tried to implement support for importComponent in test frameworks such as jest in hops.
While I had fun creating it, the resulting babel-plugin to "fix" the below mentioned issues has as many locs as the original importComponent's babel plugin.

It does not work out of the box, because importComponent relies on wepback being present.(require.resolveWeak and __webpack_modules__) are the ones in particular causing issues.

The __webpack_modules__ error ( ReferenceError: __webpack_modules__ is not defined) should be fixable by checking for its existence before accessing a value.

For require.resolveWeak, I suggest to add a test option to the babel plugin which will simply replace the require.resolveWeak with a 0 as moduleId. Afaik this should not cause any issues.

There's another issue, because the importComponent plugin adds an import statement which then won't be handled by babel-plugin-dynamic-import-node. The following error is thrown instead: Jest encountered an unexpected token I tried to shuffle plugins around, but to no avail. I suppose it's some race condition? tbh I wonder how such issues don't happen more regularly with babel plugins, but I might just not understand them well enough yet.

Maybe we could also use Promise.resolve(require('whatever')) instead of using import when the test flag is passed?

With untool/core we can use bootstrap to quickly create a mixinable container and call defined mixin methods on this container.
Unfortunately this only works for core mixins.

I need to be able to access getApolloCache from the hops-graphql package and therefore have hacked the following code:

import { environmentalize } from '@untool/core/lib/env';
import { getConfig } from '@untool/core/lib/config';
import define from 'mixinable';

process.env.UNTOOL_NSP = 'hops';
const bootstrap = (configOverrides = {}) => {
  const config = environmentalize(getConfig(configOverrides));
  const mixins = config.mixins.server.map((mixin) => require(mixin));
  const strategies = mixins.reduce(
    (result, mixin) => ({ ...result, ...mixin.strategies }),
    {}
  );
  return define(strategies, mixins)(config);
};

const mixinContainer = bootstrap((config) => {
  config.mixins = ['hops-graphql'];
  return config;
});

Maybe we could make it so that bootstrap has two arguments bootstrap(configOverrides, mixinType) where mixinType can be one of browser | server | runtime or something similar instead of hardcoding server.

Also: Not sure if this is actually a good idea, because some mixins contain code that needs to be run through webpack before it can be used..

Usecase: I need to run this in jest in order to have access to the correctly configured fragment matcher of hops-graphql.

With an increasing usage of environment variables in the configuration, it might make sense to look at how we could specify default values for them.

In my opinion the preferred approach would be to have some kind of syntax in the value field:

{
  "name": "untest",
  "license": "MIT",
  "untool": {
    "apiURL": "[API_URL='https://api.com']"
  }
}

Currently, only @dmbch, @ZauberNerd and my humble self are in the admin team of this repository and hence capable of releasing untool.

I think, we should add more people, namely @herschel666 and @jhiode to this admin team.

It seems, I can't do it as I'm not an org admin.

I'm well aware that a CI step to release would be adventurous, but since we don't have that yet, I'd argue allowing more people to release is a good workaround for now.

Because namespace is confusing, especially with our internal tools, where we also use the term namespace, but with a different meaning.
/cc @jhiode @matthias-reis

I want to be able to define alias like

const path = require('path');

module.exports = {
  "browsers": "last 1 Chrome versions",
  "alias": {
    "@": path.resolve(__dirname, 'src')
  },
  "locations": [
    "/",
    "/counter"
  ]
};

So, for example, we will be able to replace ../../../../../../../../components/Unicorn by @/components/Unicorn

We are using a new config option of @babel/preset-env which throws an error when being set on older versions of the preset, therefore we need to raise the minimum version of the preset in the package.json of @untool/webpack.

The react-router package only provides core functionality and is a direct dependency of react-router-dom which also re-exports the StaticRouter that we are using.

Therefore I'd propose to remove react-router from the list of peerDependencies to reduce the number of peer depedencies and to only use react-router-dom in our server and browser mixins.

Do not reject the build when webpack generates warnings (i.e. only reject on errors) for server-side builds.

Browserslist is used by many different tools and therefore we should remove the field "browsers" from our config and instead use the browserslist API directly because that allows our consumers to define their browsers list in any of the supported ways and ensures that other tools may also use the specified browsers list.

Given a server mixin with the following content:

class MyClass extends Mixin {
  getTemplateData(data) {
    data.assetsByType.js.push('foo.js');
    return data;
  }
}

When you load the page three times, foo.js will be found inside the markup three times instead only once (as you'd expect). The amount grows when you re-load further.

It's probably caused, because assets are always resolved with the exact same value and this code https://github.com/untool/untool/blob/master/packages/webpack/lib/middleware/assets.js#L7 only creates a shallow copy, which means that assetsByType always stays the same even for new requests.

Although the node .env package maintainers argue against doing that (admittedly with arguments I don't really seem to understand), I'd like to go the rails (and create react app) way and allow .env.development .env.local etc. overrides . https://github.com/facebook/create-react-app/blob/69c3d4b04c233ba9fb9d16691056bd09076177b1/packages/react-scripts/template/README.md#what-other-env-files-can-be-used

Imo it'll lead to a dev setup that is much easier to use and reason about, because those env files can be checked in.

What do you think @dmbch @ZauberNerd?

We noticed in Hops that we can't use environment variables when whitelisting a full object.

Example:

module.exports = {
  foo: {
    bar: '[FOOBAR]',
  },
  browserWhitelist: {
    foo: true, // does not add `FOOBAR` to the list of environment variables and `foo.bar` will be an empty string.
  },
};

It works though if we use the full path for the entry:

module.exports = {
  foo: {
    bar: '[FOOBAR]',
  },
  browserWhitelist: {
    'foo.bar': true, // this works as expected
  },
};

Root cause is that we only check for the full path (line 95):