This repository has been archived by the owner on Mar 15, 2024. It is now read-only.
Component #128
Merged
Component #128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We do not need this anymore because support for top-level await is widespread.
Previously we used to generate a standalone schema file. But this was dropped in favor of runtime dependency.
The component system has been implemented to support different execution environments in production and help with testing by providing stub/spy component instances. These goals can be achieved by a vastly simplified system. The central idea is to use search parameters in import urls. These are used by automatically generate files at the top level of each component to decide which instance to import.
The most straightforward way to encode these search parameters would be to map each component to its wanted instance -- eg: `?uuid=random&time=date`. Instead, we also provide the name of the execution environment so that we do not have to specify the instance of component which only have a single instance available for that environment -- eg: `?env=node&time=date`.
TLDR: The new component system no longer need dependency injection and uses esm import/export with search param:
```js
const { Math: { random } } = globalThis;
export default (dependencies) => {
const { time: { now } } = dependencies;
return {
getUUID: () =>
`${now.toString(36)}_${random.toString(36).substring(2)}`,
};
};
```
```js
const { URL, Math: { random } } = globalThis;
const { search } = new URL(import.meta.url);
const { now } = await import(`../../time/index.mjs${search}`);
export const getUUID = () =>
`${now.toString(36)}_${random.toString(36).substring(2)}`;
```
To help reviewing, I split in different commits the related changes in
components because they are very large and simple.
1) Change `.build` files in simpler `.env` file which do not list
dependencies.
2) Remove `.eslint.yml` files which are now automatically generated.
3) Update component prod and test files to use import/export with search parameters instead of dependency injection.
4) Update entry points in lib to import components with search parameters.
Listing dependencies is no longer required in the new component system.
These files are now generated based on `.env` files. Hence they should be removed from the version control. I used to occasion to tweak some more the ignore files.
These were implemented to support browser. But we probably want websocket anyway. YAGNI, I guess..
This commit although very large is straightforward: * In prod files: * Instead of exporting a default function that expects dependencies, we import the proxy file residing at the top-level of each component. * Instead of returning the methods of the component, we export them. * In test files: * Instead of loading prod files with dependency injection, we import them. * Instead of loading components with dependency injection, we import their proxy file. The only changes in logic are located in the url and path components because the previous system used dependency injections to test the platform-specific parts of these components.
Directly import components with search parameters instead of importing generated files in dist.
Instead of having one instance of `log` for each level, we use module parameters. And instead of setting the log file via globals we also use module parameters.
In order to test whether a file is excluded we first need to load its source map. That is because the exclusion uses the source path and not the generated path (if applicable). When the file is located in `node_modules`, the source map file is often not present. So this happens a lot in practice and is completely fine.
- Split `lib/__common__.mjs` into `lib/loader.mjs` and `lib/configuration.mjs` because these are two separate concerns. - Use global variable instead of hidden global variables. Hidden global variables are not part of the global object and cannot be introspected. That is good because they are less interfering with the observed application. But that is also bad because it is complex to use a dynamic name for these and they cannot be deleted. - Make this global variable configurable via `configuration.hooks.esm`.
Similarly to improved esm hook, we use a global variable instead of a global hidden variable and make it configurable via `configuration.hook.apply`.
Similarly to improved apply hook, we use a global variable instead of a global hidden variable and make it configurable via `configuration.hook.eval.hidden`. Now, all global variables can be configured which make coupling between different components more explicit.
Closed
dividedmind
approved these changes
Oct 6, 2022
build/component/index.mjs
Outdated
| const envs = parseEnv( | ||
| await readFileAsync(new URL(`${component}/${instance}/.env`, home), "utf8"), | ||
| ); | ||
| if (envs.every(isFirstOccurence)) { |
There was a problem hiding this comment.
I'll just note this gave me pause because it seems like an awfully pessimistic (quadratic!) way to ensure array elements are unique; I'd have expected new Set(envs).size === envs.length instead. I suppose the limited length of the arrays make it actually faster in practice, and ultimately speed doesn't really matter here anyway. Maybe it could use a comment though.
There was a problem hiding this comment.
I actually follow your suggestion, I had not thought about using Set 🧇 . Not so much that it is faster but it requires less code. 52d2ab0
|
|
||
| export const filterAsync = async (array, predicateAsync) => { | ||
| const filtered_array = []; | ||
| for (let index = 0; index < array.length; index += 1) { |
There was a problem hiding this comment.
Is there a reason to use vanilla loops instead of for ... of? I can't imagine user having deviously overridden core methods is a concern in build code.
There was a problem hiding this comment.
I need index to pass it to predicateAsync. I'm not sure that I actually use the index but filterAsync should resemble Array.prototype.filter as much as possible IMO.
|
🎉 This PR is included in version 11.5.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Revamp of the component system and some minor fixes.