[WIP] Modernise ember-leaflet #704
Conversation
- Modernisation attempts - @ember/string v3 causes a lot of pain - ember-auto-import needs bump - Why not bring leaflet directly?
- I believe v6 should work here
- Needed for ensuresafecomponent
- v18 should be reasonably adopted by now
# Conflicts: # .github/workflows/ci.yml # CONTRIBUTING.md # README.md # ember-cli-build.js # index.js # tests/dummy/app/config/environment.d.ts # tests/dummy/config/ember-try.js # tests/helpers/index.js # tsconfig.json # types/global.d.ts
| - name: Run Tests | ||
| run: ./node_modules/.bin/ember try:one ${{ matrix.try-scenario }} | ||
|
|
||
| deploy-app: |
There was a problem hiding this comment.
We shouldn't remove the deploy-app job.
There was a problem hiding this comment.
Not sure what this file is, but probably we shouldn't include files for very specific tools.
There was a problem hiding this comment.
This change completely removes everything that is interesting about the current README. Please restore the readme. 🙏
| faviconsConfig: { | ||
| path: '/ADDON_DOCS_ROOT_URL' | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we should remove the fingerprint and ember-cli-favicon options. These are important for the docs app.
|
|
||
| pathBase(packageName) { | ||
| return path.dirname(resolve.sync(packageName + '/package.json', { basedir: __dirname })); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This is a big change. I understand that import Leaflet from 'leaflet'; will take care of importing the js.
But what about the css and images? What is importing them now?
| "ember-in-element-polyfill": "^1.0.0", | ||
| "ember-render-helpers": "^0.2.0", | ||
| "fastboot-transform": "^0.1.3", | ||
| "leaflet": "^1.9.2", |
There was a problem hiding this comment.
I think leaflet should be kept as a peerDependency + devDependency.
This way, the consuming app will be able to determine what leaflet version they use.
There was a problem hiding this comment.
My question is why do we have typescript related things? Are we able to start writing TS with this?
|
🙇🏾 Thanks for your attention @miguelcobain. I am experimenting with multiple approaches now (this branch and v2 so far) and trying to land the most annoying fixes for ember-cli-addon-docs (1, 2, 3) which should remove a lot of the hacks here. This PR should've been marked as [WIP] from the start. |
|
And branch that contains relatively minimal amount of changes (but still needs ember-cli-addon-docs fixes from above): https://github.com/MichalBryxi/ember-leaflet/tree/mb/hotfixes-from-abefordisc-3 |
|
I think this branch contains way better and less intrusive changes: #705 and should be used instead. Sorry for the noise. |
Main two steps are:
Rest kidna fell on me as things were broken locally.