telekom-mms / aurelia-components Goto Github PK

@mreiche : You probably noticed the renovate bot PR. I am highly inclined to decline this, because I have no clue where it came from and uninvited guests are unwelcome to me.

I'd take it though, if...

• You ordered it because you want it.
• You have previous experience with it and can recommend it.

So... opinions?

Description

This Repository was started at a time when it was only used by a single project. By now, however, via @mreiche we are involved in two different projects with different development speeds and requirements. Of course, we'd welcome further client projects and the prospective code donors they may bring.

We should now follow the npm Good Practice to SemVer our releases by the following pattern:
MAJOR.MINOR.PATCH
with the following contract:

• MAJOR : does contain breaking changes, may be arbitrarily hard on clients to migrate
• MINOR: contains additions, may break client code if it conflicts with additions (e.g. naming conflicht, shared dependency version conflict)
• PATCH: should not contain breaking changes

We should be ready to treat defect reports re. accidental breakage as bugs with high priority and typically react by

• retraction of release on npmjs
• hotfix release

We should treat dependency updates as follows:

• non-optional dependencies: pass-through their version change
  (e.g. if any MAJOR dependency update is contained, treat the next release as MAJOR)
• devDependencies: don't care
• optional (peer) dependencies: only consider resulting usage changes e.g. in jest tests, readme examples or our own, known user code for the purpose of choosing the version
  (e.g. if a MAJOR optional peer dependency update happens, but all our code stays the same, treat the next release as FIX)

Bootstrap 5 considerably changed their popover implementation compared to 4 and there is no easy or legible way to keep the PopoverCustomAttribute compatible with both. I'd like to have Bootstrap 5 support (and no longer care about 4), so I am inclined to simply replace the implementation.

... @mreiche : Would you rather keep Bootstrap 4 as an option? In that case we'd probably have to change naming convention to include the bootstrap version number in our custom attributes, e.g. I'd commit a Popover5CustomAttribute in addition to the current one.

Added a draft:

• First commit is the minimal change,
• second commit contains an additional safeguard that would have saved a lot of time figuring out why the popovers were broken in the first place,
• third commit contains an opt-out for the second commit (which I'd like to drop but included for completeness).

I stumbled across this again while developing a PR.
In package.json "optionalPeerDependencies" are defined. According to the npm docs, there is no such thing. When running npm install in any configuration, these dependencies aren't installed.

peerDependencies and optionalDependencies exist on their own.

I'd argue that for here, peerDependencies would be just fine.
If truly optional peer dependencies are what is wanted, one needs to use peerDependenciesMeta.

I've created a PR a few weeks ago without any reaction.

The BecomeVisibleCustomAttribute uses this implementation from StackOverflow.
In the question, a more recent solution exists.
It employs the Intersection Observer API.
I would advocate for the BecomeVisibleCustomAttribute to use this as well, as it simplifies the implementation and has led to better results in some recent Use Cases of mine.

If this isn't what is wanted, I'd at least update to the getBoundingClientRect variant.

We currently use very lax settings for the Typescript transpiler, resulting in a lot of errors being silent, when instead I'd expect the compiler to catch them on-build.

Here is a list of things I'd like to activate at least:

• alwaysStrict (even without TypeScript a good idea)
• noImplicitAny (quite possibly the single most important setting, because implicit any effectively disables type checking completely - and completely undetected - whenever it happens)
• noImplicitOverride
• noImplicitThis
• noPropertyAccessFromIndexSignature
• noUncheckedIndexedAccess
• noUnusedLocals
• noUnusedParameters
• strictBindCallApply
• strictFunctionTypes
• useUnknownInCatchVariables

For reference, here is the complete list of possible options.

Momentjs is sunsetting. These are the alternatives: https://github.com/you-dont-need/You-Dont-Need-Momentjs/blob/master/README.md

My favourite: day-fns

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Pending Approval

These branches will be created by Renovate only once you click their checkbox below.

• Update major (major) (@types/jest, aurelia-webpack-plugin, jest, ts-jest, typescript, webpack-cli)

Pending Status Checks

These updates await pending status checks. To force their creation now, click the checkbox below.

• Update dependency webpack to v5.92.0

Detected dependencies

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

Currently the "CacheService" does not perform the service of dropping outdated keys. Instead this must happen from the outside via invalidate. This means that keys may spontaneously be deleted silently and at any time.

return this._cacheContainer[key][1] = Promise.resolve(object); then fails because this._cacheContainer[key] is undefined.

Workaround: Always optional-chain.

Proper solution: Let the CacheService do the dropping, such that it may handle pending replies to dropped keys appropriately.

Design issue: How do we handle responses which only arrive after their key has already timed out. Either/or:

1. What we actually want to cache is values, not keys. We should TTL the values, not the keys. Only when the key is written to should TTL start counting down for it.
2. When a key times out before the value is written, discard the write (and emit a warning, I guess): This cache is not properly configured to handle the response times encountered, so they will never be cached.

... I prefer 1). @mreiche : Opinions?

Steps to reproduce:

1. make a call 0 that takes longer to resolve than TTL
2. make a call 1 after TTL of call 0 that resolves within 1 + <now of call 1> - <now of call 0>
3. make a call 2 after call 0 and 1 have both resolved

Expected: Receive result of call 1, call 2 is not executed.
Observed: Call 2 finds validUntil of call 0 and is executed; receive result of call 2.

Test case here

When the custom attribute "popover" is used on a webpage, Chrome shows the following warning:

Links:
https://html.spec.whatwg.org/multipage/dom.html#custom-data-attribute [1]
https://chromestatus.com/feature/5463833265045504 [2]
https://html.spec.whatwg.org/multipage/popover.html [3]

Once browsers start supporting the API, we'll face a namespace conflict.

@martingrossmann and I suggest:
Renaming the attribute to "data-popover" or "data-ac-popover" (ac for aurelia-components) in accordance with [1].
This is a breaking API change on our side, so the old attribute should stay available, although deprecated.

Description

Heads-up for @mreiche - feel free to close after perusing.

I am leaving the organisation at the end of the year. As a result:

• added @conradstrassburger in role Maintain (Pending Invite)
• on 2022-12-09 will
  • downgrade myself to role Read (same as yours)
  • have @martingrossmann remove me from the npmjs organisation

The Release 1.0.17 is already released on NPM: https://www.npmjs.com/package/t-systems-aurelia-components and done before the last merge, which should be 1.0.18.

Description

SortValueConverter will only sort by string or number values at the moment.

Implementing and adding a true/false sort should be easy enough and satisfy use cases.

Additional information

...