Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

fix: support legal comments #143

Merged

Conversation

JeanMeche
Copy link
Contributor

The Angular CLI would like to support critters in legal comments. Legal comments have a leading ! and aren't removed at build times.

Cf https://esbuild.github.io/api/#legal-comments

cc @alan-agius4

@@ -477,7 +477,14 @@ export default class Critters {
ast,
markOnly((rule) => {
if (rule.type === 'comment') {
const comment = rule.text.trim();
let comment = rule.text;
Copy link
Contributor

@alan-agius4 alan-agius4 Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: The following should be more performant.

const crittersComment = rule.text.match(/^!? ?critters:(.*)/);
const command = crittersComment && crittersComment[1];

Removes the need for multiple .startsWith, .trim .slice, and .replace and also reduces memory allocation.

@JeanMeche JeanMeche force-pushed the support-legal-comments branch from 231d713 to 27446ff Compare October 19, 2023 21:24
Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JeanMeche JeanMeche force-pushed the support-legal-comments branch from 27446ff to 6d5072f Compare March 5, 2024 09:24
@janicklas-ralph
Copy link
Collaborator

There are some tests failing. Good to merge after they are fixed

@JeanMeche JeanMeche force-pushed the support-legal-comments branch from 6d5072f to afb2157 Compare March 5, 2024 20:32
@JeanMeche
Copy link
Contributor Author

@janicklas-ralph We should be good now.

@janicklas-ralph janicklas-ralph merged commit 8494a53 into GoogleChromeLabs:main Mar 5, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants