-
Notifications
You must be signed in to change notification settings - Fork 106
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 Ember #2438
base: main
Are you sure you want to change the base?
Add Ember #2438
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
self-review -- need to make some changes
ready for review <3 |
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 looks significantly different from the other tests.
We use a (very jank) karma config that records test results and compares them against the expected results, in order to maintain a number of invariants, e.g. that we're running the same number of tests across all libraries.
Having a custom test stack across different libraries will impose a maintenance burden that I'm not super excited about. Would it be possible to compile this code to standard JS so that it runs in the same karma config as the other libraries?
<script src="/@embroider/core/vendor.js"></script> | ||
<script type="module"> | ||
import Application from "#src/app"; | ||
import environment from "#config"; |
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.
Here and elsewhere, what do these nonstandard import specifiers that start with #
mean? What part of the build system makes them work? Could you instead write javascript here, with ./dist/config.js
or similar?
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 just how package.json#imports works.
A super helpful feature when you enable type=module.
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.
They are standard: https://nodejs.org/api/packages.html#subpath-imports
I can build it, yeah - is there a test project made that i can point at built files? I'll happily delete my port of the tests
Back when i first did this, it looked like there was a bunch of test duplication everywhere? Is this still the case? I can double check next i revisit this repo. UPDATE: i looked again -- all tests are copied to every library/framework and then modified pretty heavily in each one. (use of JSX vs not, different ways of rendering, instantiating, interacting, etc) |
We use |
figured it outwith I've tried in package.json:
and
neither work, # with link
❯ npm i
npm error code EUNSUPPORTEDPROTOCOL
npm error Unsupported URL Type "link:": link:../__shared__/webcomponents
npm error A complete log of this run can be found in: /home/nvp/.npm/_logs/2025-03-13T00_06_23_959Z-debug-0.log # with file
❯ npm i
npm error Cannot read properties of null (reading 'matches')
npm error A complete log of this run can be found in: /home/nvp/.npm/_logs/2025-03-13T00_06_57_448Z-debug-0.log I can't even add the folder ❯ npm add ../__shared__/webcomponents
npm error Cannot read properties of null (reading 'matches')
npm error A complete log of this run can be found in: /home/nvp/.npm/_logs/2025-03-13T00_08_28_746Z-debug-0.log Here are my versions ❯ npm -v
10.9.0
custom-elements-everywhere/libraries/ember on add-ember [✘!]
❯ node -v
v23.6.0 My issue was just that the repo was dirty, and |
It looks like all of the tests are custom already? what do you mean? I checked, angular, svelte, solid, react, and they're all custom. |
these are also custom for every library/framework. Would it make sense for the test runner move to a standard reporter such as TAP? so the test implementation doesn't matter so much? 🤔 mocha already supports TAP: https://mochajs.org/#tap text output of `npm test` using TAP
|
Resolves #237
[ ] Integrate into reporting / infratests are manually ran?Of note, ember currently uses Vite, not webpack
old notes
, so we have to make some tweaks to shoehorn a vite build into this repo's way of using webpack -- shouldn't be too bad tho.