Skip to content
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

feat: officially expose testing helpers #5879

Conversation

EisenbergEffect
Copy link
Contributor

@EisenbergEffect EisenbergEffect commented Apr 22, 2022

Pull Request

📖 Description

This PR fixes up the organization of the testing helpers in foundation and exposes them through the exports configuration of the package.json.

🎫 Issues

👩‍💻 Reviewer Notes

@nicholasrice Can you check that I've handled the package.json correctly? I didn't rename the index.ts file for the main package. I just tried to fix up the testing helpers a bit based on what you were doing in the ssr package.

📑 Test Plan

No changes to functionality. Existing tests, including those that use these helpers, still pass.

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

We should probably follow this up with a change to adopt the "exports" naming scheme and set up all the package.json files properly to use this feature. We may want to leverage a development and production export configuration for fast-element as well.

EisenbergEffect and others added 6 commits April 12, 2022 21:45
* refactor: clean up array observer

* refactor: modernize the splice code

* chore: set up SSR package (#5589)

* project files and starting to set up test infrastructure

* incorporating ts project references and getting tests working

* adding .npmignore to ingore tests and server

* adding readmes

* Update packages/web-components/fast-ssr/package.json

Co-authored-by: Chris Holt <[email protected]>

Co-authored-by: nicholasrice <[email protected]>
Co-authored-by: Chris Holt <[email protected]>

* feat: enable multiple instances of fast-element on a page at once (#5718)

* feat: enable multiple instances of fast-element on a page at once

* Change files

Co-authored-by: EisenbergEffect <[email protected]>

* refactor: extract polyfill and polyfill-like code to an optional module (#5752)

* refactor: extract polyfill and polyfill-like code to an optional module

* Change files

* fix: correct build break in fast-foundation from removing $global

* fix: update templates to use classList and fix classList bug

* Change files

* feat: require trusted types for bindings to innerHtml & move core policy

* fix: bug that arises when there are real trusted types

* refactor: minor internal cleanup in the polyfills

* fix: remove polyfill external module dependency

Co-authored-by: EisenbergEffect <[email protected]>

* feat: implement template renderer infrastructure  (#5698)

* implement factory fn to create TemplateRenderer and ElementRenderer

* rename index to exports

* adding default render info object

* adding initial directive rendering

* adds initial custom element rendering

* add directive renderers

* add renderInfo to DirectiveRenderer

* use default directive renderers

* adding attribute binding tests

* ensure attributes don't get emitted twice for custom elements

* Adding internal function to render op codes so that we can omit template elements when rendering custom element templates

* emit template open and close codes from template parser

* implement FAST parser changes and fix tests

* working AspectedHTMLDirective changes into branch

* naive implementation of custom element template rendering

* adds rendering of template op code

* progress on elmeent rendering

* fixing custom element attribute rendering bug

* Change files

* adding boolean and property binding test

* fix bug preventing template elements from being parsed

* add failing template nesting test

* update parsing of content nodes to correctly render interpolated templates

* fix API report and formatting

* adding repeater rendering

* adding directive implementations for Ref, Slotted, and Children

* clean up op types to reduce rendant information

* remove commented code that is no longer needed

* adding tests for ref, slotted, and children

* re-use noop function for noop directives

* cache length of arrays in for loops

* adding style tests

* implementing style renderer

* remove 'data' from style id name

* adding tests

* renaming files to align with current conventions

* update attachShadow signature

* adding classList support

* fixing tests and supporting string types

* removing comment that is a non-concern

* add playwright install step to ci

* add browser path to ci script

* change install script to install at the package level

* update playwright

* Change files

* fixing test

Co-authored-by: nicholasrice <[email protected]>

* feat: new execution context design (#5800)

* refactor: new design for execution context

* feat: add two new event helpers to the execution context and tests

* fix: wip update types to match new context apis

* fix: update foundation and components template types

* Change files

* fix: update template type in fast-website

* fix: update site components for new template types

* fix: add missing api updates

Co-authored-by: EisenbergEffect <[email protected]>

* feat: new HTMLDirective API

* feat: new directive registration/identification model

* refactor: refine design/implementation of new directive aproach

* refactor: clean up comments, interfaces, types for directives/registries

* fix: update foundation to new APIs

* fix: update router to new directive APIs.

* fix: update ssr to new directive APIs

* refactor: clean up ssr switch to new directive APIs

* fix: update React wrapper lib to use latest APIs

* Change files

* fix: post rebase issues

* fix: update reflectAttributes directive to new directive APIs

* test: add more tests to capture new html/directive aspect scenarios

* Change files

* chore: run prettier on foundation

* Update packages/web-components/fast-ssr/src/template-parser/template-parser.spec.ts

Co-authored-by: Nicholas Rice <[email protected]>

* Update packages/web-components/fast-ssr/src/template-parser/template-parser.spec.ts

Co-authored-by: Nicholas Rice <[email protected]>

* Update packages/web-components/fast-foundation/src/directives/reflect-attributes.ts

Co-authored-by: Nicholas Rice <[email protected]>

* Update packages/web-components/fast-foundation/src/directives/reflect-attributes.ts

Co-authored-by: Nicholas Rice <[email protected]>

* chore: cleanup tests after rebase

Co-authored-by: Nicholas Rice <[email protected]>
Co-authored-by: nicholasrice <[email protected]>
Co-authored-by: Chris Holt <[email protected]>
Co-authored-by: EisenbergEffect <[email protected]>
* expose dom shim from package root

* ensure DOM updates are made synchronously

* upgrade lit to use latest dom shim

* fleshing out dom-shim with foundation-element requirements

* fix root <template> behavior to be more durable to formatting

* implement classList binding for native elements

* enables style retrevial for a custom element through SSR style-strategy

* clean up exports

* add tagName to elements created by ElementRenderer to be used by componentPresentation

* prettier

Co-authored-by: nicholasrice <[email protected]>
* feat: new CSSDirective design

* feat: attach the partial helper to the css helper

* fix: update foundation to new CSSDirective API

* fix: update components to new CSSDirective API

* docs: add missing docs to CSSTEmplateTag type

* Change files

* fix: add back cssPartial with deprecation message

Co-authored-by: EisenbergEffect <[email protected]>
* rename title attribute on disclosure to summary

* Change files
* update the default value of dialog  modal attribute to false

* Change files
@EisenbergEffect EisenbergEffect linked an issue Apr 22, 2022 that may be closed by this pull request
@EisenbergEffect
Copy link
Contributor Author

@nicholasrice Any idea why TS can't find the declaration file now? I just changed the folder and file name. I did see that the exports package.json feature requires TS 4.5, but I'm not actually using that for the import here. Thoughts?

@@ -19,6 +19,10 @@
"types": "dist/fast-foundation.d.ts",
"unpkg": "dist/fast-foundation.min.js",
"type": "module",
"exports": {
"." : "./dist/esm/index.js",
"./testing": "./dist/esm/testing/exports.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I really like this feature

@nicholasrice
Copy link
Contributor

@nicholasrice Any idea why TS can't find the declaration file now? I just changed the folder and file name. I did see that the exports package.json feature requires TS 4.5, but I'm not actually using that for the import here. Thoughts?

I think this is because in the react-wrapper package you are importing from a path that isn't enumerated in the "exports" field of the fast-foundation/package.json (@microsoft/fast-foundation/dist/esm/testing/exports). That should really import from @microsoft/fast-foudation/testing, however I think you'll run into an issue w/ that as well because of our TypeScript version. This may be blocked until we can upgrade TypeScript

@EisenbergEffect
Copy link
Contributor Author

@nicholasrice Any idea why TS can't find the declaration file now? I just changed the folder and file name. I did see that the exports package.json feature requires TS 4.5, but I'm not actually using that for the import here. Thoughts?

I think this is because in the react-wrapper package you are importing from a path that isn't enumerated in the "exports" field of the fast-foundation/package.json (@microsoft/fast-foundation/dist/esm/testing/exports). That should really import from @microsoft/fast-foudation/testing, however I think you'll run into an issue w/ that as well because of our TypeScript version. This may be blocked until we can upgrade TypeScript

I wondered if it was something like that...this can easily wait to merge until we can update that. I just want to make sure we've got all this patched up along with the TS version before we start publishing beta packages.

Note: This won't work until we update TypeScript to at least 4.5.
@nicholasrice
Copy link
Contributor

Yeah if the exports field is enumerated, Node will prevent import drilling into the underlying file structure, which is actually pretty nifty, it provides a lot of control over the package structure and helps prevent access to code intended to be private. But it definitely breaks older versions of TypeScript.

@nicholasrice nicholasrice force-pushed the features/fast-element-2 branch from 9739096 to 74c0c9b Compare May 4, 2022 17:48
@nicholasrice nicholasrice force-pushed the features/fast-element-2 branch 2 times, most recently from 8d37630 to caaa685 Compare May 9, 2022 19:19
@EisenbergEffect
Copy link
Contributor Author

Superseded by #5958

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Publicly Expose fixture<T> in @microsoft/fast-tooling
5 participants