Skip to content

project: added project structure with tests, benchmarks, ci and linting#1

Merged
palazzem merged 5 commits into
masterfrom
project-structure
Jan 22, 2018
Merged

project: added project structure with tests, benchmarks, ci and linting#1
palazzem merged 5 commits into
masterfrom
project-structure

Conversation

@rochdev

@rochdev rochdev commented Jan 19, 2018

Copy link
Copy Markdown
Member

This PR creates the project structure with the following features:

  • Linting using ESLint
  • .editorconfig support
  • Unit testing support using mocha + chai
  • OpenTracing API conformance tests
  • Benchmarks
  • CircleCI configuration
  • Development documentation

@rochdev
rochdev requested a review from palazzem January 19, 2018 17:40
@palazzem palazzem changed the title [WIP] added project structure with tests, benchmarks, ci and linting project: added project structure with tests, benchmarks, ci and linting Jan 19, 2018
@palazzem palazzem added this to the 0.1.0 milestone Jan 19, 2018

@palazzem palazzem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great structure that we can use! All looks good to me!

@palazzem
palazzem merged commit 86fd2ea into master Jan 22, 2018
@palazzem
palazzem deleted the project-structure branch January 22, 2018 12:19
alrosot pushed a commit to alrosot/dd-trace-js that referenced this pull request May 20, 2021
* Update README and package.json

* update docs

* try to fix the build

* Update README.md

Co-Authored-By: alrex <[email protected]>

* Update package.json

Co-Authored-By: alrex <[email protected]>

* Update package.json

Co-Authored-By: alrex <[email protected]>
@tlhunter
tlhunter restored the project-structure branch December 19, 2023 20:10
@tlhunter
tlhunter deleted the project-structure branch December 19, 2023 20:10
crysmags added a commit that referenced this pull request May 14, 2026
After merging master, #8309's perf wins target functions that either no
longer exist (wrapExecute / wrapFields / pathToArray / finishResolvers
moved out of instrumentations/graphql.js) or live in a deleted file
(resolve.js was folded into execute.js). Port the still-applicable
changes onto the migrated execute plugin:

  #1 _startResolveSpan uses info.fieldNodes[0] instead of .find(fn =>
     fn.kind === 'Field'). The executor only hands FieldNode siblings.
  #2 pathToArray switches to count-then-fill (drops the trailing
     .reverse() and the push-grow).
  #3 getResolverInfo lazy-allocates resolverVars: skips both the
     up-front {} and the directive-loop assignment when args is empty
     and no directives have arguments.
  #4 shouldInstrument early-returns on config.depth < 0 (no-limit, the
     default-enabled case) before walking the path.
  #5 buildCollapsedPathString builds the collapsed key in one walk;
     drops the intermediate collapsedPath array. iastResolveCh.publish
     now passes raw path / pathString, matching the contract the old
     resolve.js had with the IAST subscriber.
  drive-by  wrapFields iterates Object.values(type._fields) directly.

  #7 (addVariableTags early-return) already auto-merged during the
     master merge.
  #6 (single documentSources.get in wrapExecute) is already the shape
     of bindStart in the migrated plugin.

The reverted depth=0 bindStart short-circuit (156a557 / ca2d041)
sat at a different position — inside bindStart, gated on _depthDisabled
(config.depth === 0). It was rolled back because piling it on top of
the resolveAsync depth-flag fix didn't move plugin-graphql-long-with-
depth-off any further. This port stays inside shouldInstrument, fires
on config.depth < 0 (the *enabled* default), and targets the hot path
on plugin-graphql-long.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
crysmags added a commit that referenced this pull request May 19, 2026
After merging master, #8309's perf wins target functions that either no
longer exist (wrapExecute / wrapFields / pathToArray / finishResolvers
moved out of instrumentations/graphql.js) or live in a deleted file
(resolve.js was folded into execute.js). Port the still-applicable
changes onto the migrated execute plugin:

  #1 _startResolveSpan uses info.fieldNodes[0] instead of .find(fn =>
     fn.kind === 'Field'). The executor only hands FieldNode siblings.
  #2 pathToArray switches to count-then-fill (drops the trailing
     .reverse() and the push-grow).
  #3 getResolverInfo lazy-allocates resolverVars: skips both the
     up-front {} and the directive-loop assignment when args is empty
     and no directives have arguments.
  #4 shouldInstrument early-returns on config.depth < 0 (no-limit, the
     default-enabled case) before walking the path.
  #5 buildCollapsedPathString builds the collapsed key in one walk;
     drops the intermediate collapsedPath array. iastResolveCh.publish
     now passes raw path / pathString, matching the contract the old
     resolve.js had with the IAST subscriber.
  drive-by  wrapFields iterates Object.values(type._fields) directly.

  #7 (addVariableTags early-return) already auto-merged during the
     master merge.
  #6 (single documentSources.get in wrapExecute) is already the shape
     of bindStart in the migrated plugin.

The reverted depth=0 bindStart short-circuit (156a557 / ca2d041)
sat at a different position — inside bindStart, gated on _depthDisabled
(config.depth === 0). It was rolled back because piling it on top of
the resolveAsync depth-flag fix didn't move plugin-graphql-long-with-
depth-off any further. This port stays inside shouldInstrument, fires
on config.depth < 0 (the *enabled* default), and targets the hot path
on plugin-graphql-long.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
christophe-papazian added a commit that referenced this pull request Jun 16, 2026
- #3 (regression from round 5): constraintMatchesSlash no longer treats a '/' that
  appears only inside a character class as slash-spanning. `[^/]+` denies slashes, so
  /:id([^/]+)/users (Express 4) normalizes to /{id}/users again instead of null.
- #2: a non-terminal catch-all (a wildcard with a non-empty segment after it, incl. an
  optional {/*rest}/tail) is rejected at compile (→ null) rather than silently dropped.
- #1: a segment containing two independent optional groups (e.g. :a{.:b}{-:c}) is
  rejected (→ null); our single per-segment regex can't replicate path-to-regexp's
  ordered-alternative assignment, so the combined name could be wrong. Single
  intra-segment optionals (:id{.:format}) are unaffected.
- #4: structural-only optional groups — a {...} wrapping only nested group(s) with no
  segment/token of its own (e.g. the outer braces in /a{{/b}}) — are collapsed by
  reparenting represented groups to their nearest represented ancestor, so /a{{/b}}
  resolves to /a/b | /a instead of always /a.

Adds a regression test per finding (verified against live Express 5).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants