Covert to V2 addon (a real npm package!) #345

NullVoxPopuli · 2022-07-05T23:22:30Z

Changelog notes

BREAKING
- requires ember-auto-import@v2 or embroider

(otherwise there is nothing changed about how @ember/string works and the two files involved (index.ts and cache.ts) show up as moves)

Changes

Much of this was done via ember init -b @embroider/blueprint --pnpm --typescript

There is some divergence, however (@ember/string is maybe not the best example of a use case for the v2 addon blueprint):

ci.yml / ember-try support is expanded back to ember 3.20
I removed @types/ember__string from the test-app, because we build the types here
I removed glint because this package does not need that tool, and tsc is sufficient
I removed linting from the test command in the test-app. it's silly as we have a separate lint command in CI having lint in the test command made the top-level pnpm test command do too much. Now test just means test.
tsconfig.json has additional settings set because declaration emit is kind of wonkey and we may want to consider adding a separate @tsconfig/ember (maybe @tsconfig/esm-library?) because @tsconfig/ember is currently splitting duty between libraries and apps, and trying to make both happy, even though apps are non-spec compliant, for the most part)
vitest for tests (in addition to the test-app (this was done in the original pass at conversion back in 2022))
- this is because @ember/string is legit nothing special and requires no ember APIs. For folks working in this repo, this means they'll have instant test feedback and better editor integration when working in tests
- If maintainers don't want this, I can move all the tests back to the ember app, but I think it's good to have as an example for certain types of utility libraries
- the test app is stictly to check that the v2 addon boilerplate is correctly configured (addon-main.cjs, keywords: [ember-addon], etc).

Things we could do since @ember/string has no ember api usage

make it a real ESM package (type=module), remove keywords: [ember-addon], remove package.json#ember-addon, etc
for the type of library that @ember/string is, the v2 addon format is more boilerplate plate than we need and sets us back a bit from where we could be (still ahead of a v1 addon though!)

Unrelated changes

tests are now in TypeScript

kategengler · 2022-07-21T18:57:49Z

.github/workflows/ci-build.yml

 jobs:
  test:
-    name: Tests
+    name: "Tests"
    runs-on: ubuntu-latest

    steps:


Did we lose testing against multiple node versions here?

I forgot to answer this!

There is no need, because the output of this repo is browser-code, so node isn't relevant.

package.json

NullVoxPopuli · 2022-07-21T21:17:03Z

Gonna pull some smaller PRs out of this, since folks are interested in @ember/string again 🥳

I also want to use this as an opportunity to see how v2 addon conversions work.
It's not all that much, but it is precise.

(if a maintainer wants to convert this PR back to "Draft", that'd be appreciated <3 )

The order will be (and I'll link to these PRs as they're created):

switch to pnpm
convert to monorepo
extract dummy / tests app to its own package / app (adding typescript to the tests app might be a separate PR, I'm not sure yet)
convert addon to v2 addon
convert core test suite to vitest (way faster than booting up an ember app, and we don't interact with any ember apis) -- the ember app will still stay (like it is in this PR, but it'll have fewer tests)

As this happens, the "diff" of this PR will get much smaller (even though most of it is lockfile, and js->ts conversions).
I don't think we need to do any releasing throughout this process.

NullVoxPopuli · 2024-03-01T23:21:31Z

ember-string/src/index.ts

basically an original file!

NullVoxPopuli · 2024-03-01T23:22:24Z

ember-string/src/index.ts

  ) => initialChar + (chr ? chr.toUpperCase() : '');
  const parts = str.split('/');

  for (let i = 0; i < parts.length; i++) {
-    parts[i] = parts[i]
+    parts[i] = (parts as any)[i]


added because TS has gotten stricter since this PR was initially opened and TS now considers this dangerous -- it would be great though if it could determine that i was bounded to the range within parts

NullVoxPopuli · 2024-03-01T23:22:43Z

ember-string/src/index.ts

      .replace(STRING_CLASSIFY_REGEXP_1, replace1)
      .replace(STRING_CLASSIFY_REGEXP_2, replace2);
  }

  return parts
    .join('/')
    .replace(STRING_CLASSIFY_REGEXP_3, (match /*, separator, chr */) =>
-      match.toUpperCase()
+      match.toUpperCase(),


newer prettier version wanted commas 🤷

NullVoxPopuli · 2024-03-01T23:24:00Z

ember-string/tsconfig.json

+  "compilerOptions": {
+    "allowJs": true,
+    "declarationDir": "declarations",
+    "noEmit": false,


these 3 settings here were required to generate declarations with tsc --declaration.

I need to look in to if it makes sense to put these in the addon-blueprint, or if we should just "fork" @tsconfig/ember at this point (still in tsconfig/bases tho) maybe with a v2 app the delta can start shrinking again

Add LTS 5.4 Run ember init to sync with blueprint Lint:fix Lint:fix Fix package.json Fix types build tsc args! Can we have the same compatibility as before? Re-add the tests workspace The test-app does not need to lint in order to test Try to re-gain ember 3.20+ compat Drop support for ember < 3.28 delete more glint stuff

test-app/ember-cli-build.js

NullVoxPopuli force-pushed the v2-addon branch from 6c873e9 to f4cd4d7 Compare July 9, 2022 00:36

NullVoxPopuli marked this pull request as ready for review July 9, 2022 02:12

kategengler reviewed Jul 21, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

NullVoxPopuli mentioned this pull request Jul 21, 2022

Switch to pnpm #348

Open

This was referenced Aug 9, 2022

Add --vitest flag embroider-build/addon-blueprint#58

Closed

Convert to V2 Addon so I can use this in Vite emberjs/ember-qunit#956

Closed

This was referenced Aug 25, 2022

Convert to TS or supply ambient definitions ember-cli/ember-page-title#239

Closed

v2 addon conversion? machty/ember-concurrency#480

Closed

NullVoxPopuli mentioned this pull request Oct 2, 2022

V2 Addon conversion? tracked-tools/ember-async-data#351

Closed

NullVoxPopuli force-pushed the v2-addon branch from 45a52e6 to 0233f21 Compare March 1, 2024 16:29

NullVoxPopuli commented Mar 1, 2024

View reviewed changes

ember-string/src/index.ts

Copy link

Contributor Author

NullVoxPopuli Mar 1, 2024

Choose a reason for hiding this comment

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

basically an original file!

NullVoxPopuli commented Mar 1, 2024

View reviewed changes

NullVoxPopuli force-pushed the v2-addon branch from 830c1b9 to 87c787f Compare March 1, 2024 23:32

NullVoxPopuli commented Mar 1, 2024

View reviewed changes

test-app/ember-cli-build.js Outdated Show resolved Hide resolved

NullVoxPopuli added 2 commits March 1, 2024 18:45

Update test-app/ember-cli-build.js

57091b5

Bring back support for 3.24 and 3.20

5ec4703

NullVoxPopuli mentioned this pull request Mar 4, 2024

(Real™ package™)™ #425

Merged

Delete unused files

cf244c0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Covert to V2 addon (a real npm package!) #345

Covert to V2 addon (a real npm package!) #345

NullVoxPopuli commented Jul 5, 2022 •

edited

Loading

kategengler Jul 21, 2022

NullVoxPopuli Jan 23, 2023

NullVoxPopuli commented Jul 21, 2022 •

edited

Loading

NullVoxPopuli Mar 1, 2024

NullVoxPopuli Mar 1, 2024

NullVoxPopuli Mar 1, 2024

NullVoxPopuli Mar 1, 2024

Covert to V2 addon (a real npm package!) #345

Are you sure you want to change the base?

Covert to V2 addon (a real npm package!) #345

Conversation

NullVoxPopuli commented Jul 5, 2022 • edited Loading

Changelog notes

Changes

Unrelated changes

kategengler Jul 21, 2022

Choose a reason for hiding this comment

NullVoxPopuli Jan 23, 2023

Choose a reason for hiding this comment

NullVoxPopuli commented Jul 21, 2022 • edited Loading

NullVoxPopuli Mar 1, 2024

Choose a reason for hiding this comment

NullVoxPopuli Mar 1, 2024

Choose a reason for hiding this comment

NullVoxPopuli Mar 1, 2024

Choose a reason for hiding this comment

NullVoxPopuli Mar 1, 2024

Choose a reason for hiding this comment

NullVoxPopuli commented Jul 5, 2022 •

edited

Loading

NullVoxPopuli commented Jul 21, 2022 •

edited

Loading