-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add build-time code exclusion using code fencing #12060
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
6d9512e
Initial code fencing transform implementation
rekmarks 779974a
Rename some stuff
rekmarks 6f97fa6
Concatenate file buffers before applying transform
rekmarks ac3d520
Begin adding tests
rekmarks 3e7fbdb
Complete tests, reconfigure Jest
rekmarks 6cdb3a8
Rename fileName parameter to filePath
rekmarks f6126d1
Lint transformed files, init lint tests
rekmarks 28c649d
Finish transformed linting tests
rekmarks a90fb76
Add spaced-comment marker for fences
rekmarks 999250c
Add build system arg for toggling fence linting
rekmarks 20bccdd
Replace through2 with a Transform implementation
rekmarks 2622421
Default shouldLintFenceFiles to false for dev builds
rekmarks c41078a
Fix spaced-comment config
rekmarks a950ea2
Update LavaMoat policy
rekmarks 09ca404
fixup! Update LavaMoat policy
rekmarks f74378d
Check that the linter is called correctly in tests
rekmarks 5bd2a12
Add transform function docstrings
rekmarks bde0241
Add some directive test cases
rekmarks ae4ecde
Forbid empty fences
rekmarks b7545fe
Add documentation, handle some parsing edge cases
rekmarks ff246dd
Remove linting
rekmarks b0c42ec
Tweak a test
rekmarks efd8cd3
Stop using 'key in object'
rekmarks 0828289
Address review feedback
rekmarks 073f7cf
Fix SC2048 violation
rekmarks dfadb27
Update development/build/transforms/remove-fenced-code.js
rekmarks ce15bf2
Update development/build/transforms/remove-fenced-code.js
rekmarks 474ec02
Apply suggestions from code review
rekmarks 662f5ae
Fix test scripts
rekmarks 892a0a1
Fix tests and some bugs
rekmarks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
# Local Browserify Transforms | ||
|
||
This directory contains home-grown Browserify transforms. | ||
Each file listed here exports a transform function factory. | ||
|
||
## Removing Fenced Code | ||
|
||
> `./remove-fenced-code.js` | ||
|
||
When creating builds that support different features, it is desirable to exclude | ||
unsupported features, files, and dependencies at build time. Undesired files and | ||
dependencies can be excluded wholesale, but the _use_ of undesired modules in | ||
files that should otherwise be included – i.e. import statements and references | ||
to those imports – cannot. | ||
|
||
To support the exclusion of the use of undesired modules at build time, we | ||
introduce the concept of code fencing to our build system. Our code fencing | ||
syntax amounts to a tiny DSL, which is specified below. | ||
|
||
The transform concatenates each file into a single string, and a string parser | ||
identifies any fences in the file. If any fences that should not be included in | ||
the current build are found, the fences and the lines that they wrap are | ||
deleted. The transform errors if a malformed fence line is identified. | ||
|
||
For example, the following fenced code: | ||
|
||
```javascript | ||
this.store.updateStructure({ | ||
..., | ||
GasFeeController: this.gasFeeController, | ||
TokenListController: this.tokenListController, | ||
///: BEGIN:ONLY_INCLUDE_IN(beta) | ||
PluginController: this.pluginController, | ||
///: END:ONLY_INCLUDE_IN | ||
}); | ||
``` | ||
|
||
Is transformed to the following if the build type is not `beta`: | ||
|
||
```javascript | ||
this.store.updateStructure({ | ||
..., | ||
GasFeeController: this.gasFeeController, | ||
TokenListController: this.tokenListController, | ||
}); | ||
``` | ||
|
||
Note that multiple build types can be specified by separating them with | ||
commands inside the parameter parentheses: | ||
|
||
```javascript | ||
///: BEGIN:ONLY_INCLUDE_IN(beta,flask) | ||
``` | ||
|
||
### Code Fencing Syntax | ||
|
||
> In the specification, angle brackets, `< >`, indicate required tokens, while | ||
> straight brackets, `[ ]`, indicate optional tokens. | ||
> | ||
> Alphabetical characters identify the name and purpose of a token. All other | ||
> characters, including parentheses, `( )`, are literals. | ||
|
||
A fence line is a single-line JavaScript comment, optionally surrounded by | ||
whitespace, in the following format: | ||
|
||
```text | ||
///: <terminus>:<command>[(parameters)] | ||
|
||
|__| |________________________________| | ||
| | | ||
| | | ||
sentinel directive | ||
``` | ||
|
||
The first part of a fence line is the `sentinel`, which is always the string | ||
"`///:`". If the first four non-whitespace characters of a line are not the | ||
`sentinel`, the line will be ignored by the parser. The `sentinel` must be | ||
succeeded by a single space character, or parsing will fail. | ||
|
||
The remainder of the fence line is called the `directive`. | ||
The directive consists of a `terminus`, `command`, and (optionally) `parameters`. | ||
|
||
- The `terminus` is one of the strings `BEGIN` and `END`. It must be followed by | ||
a single colon, `:`. | ||
- The `command` is a string of uppercase alphabetical characters, optionally | ||
including underscores, `_`. The possible commands are listed later in this | ||
specification. | ||
- The `parameters` are a comma-separated list of RegEx `\w` strings. They must | ||
be parenthesized, only specified for `BEGIN` directives, and valid for the | ||
command of the directive. | ||
|
||
A valid code fence consists of two fence lines surrounding one or more lines of | ||
non-fence lines. The first fence line must consist of a `BEGIN` directive, and | ||
the second an `END` directive. The command of both directives must be the same, | ||
and the parameters (if any) must be valid for the command. | ||
|
||
If an invalid fence is detected, parsing will fail, and the transform stream | ||
will end with an error. | ||
|
||
### Commands | ||
|
||
#### `ONLY_INCLUDE_IN` | ||
|
||
This, the only command defined so far, is used to exclude lines of code | ||
depending on the type of the current build. If a particular set of lines should | ||
only be included in a particular build type, say `beta`, they should be wrapped | ||
as follows: | ||
|
||
```javascript | ||
///: BEGIN:ONLY_INCLUDE_IN(beta) | ||
console.log('I am only included in beta builds.'); | ||
///: END:ONLY_INCLUDE_IN | ||
``` | ||
|
||
At build time, the fences and the fenced lines will be removed if the build is | ||
not `beta`. | ||
|
||
Parameters are required for this command, and they must be provided as a | ||
comma-separated list of one or more of: | ||
|
||
- `main` (the build system default build type) | ||
- `beta` | ||
- `flask` |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is some incredible documentation by the way. Not just this but the inline docs too.
I was expecting this transform to be difficult to understand, but it was really straightforward with all of these explanations at each step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1000