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

chore(deps): remove unused AWS SDK for JavaScript #2511

Merged
merged 1 commit into from
Feb 20, 2024
Merged

chore(deps): remove unused AWS SDK for JavaScript #2511

merged 1 commit into from
Feb 20, 2024

Conversation

trivikr
Copy link
Contributor

@trivikr trivikr commented Nov 15, 2023

Description

From AWS SDK for JavaScript v2 README:

We are formalizing our plans to make the Maintenance Announcement (Phase 2) for AWS SDK for JavaScript v2 in 2023.

The dependency was added in #535 to upload panes.
It was not removed in #2103

@trivikr
Copy link
Contributor Author

trivikr commented Nov 15, 2023

cc @RobbieTheWagner

@RobbieTheWagner
Copy link
Member

@trivikr we used to use this to upload to s3 I think. Are we not doing that anymore?

@trivikr
Copy link
Contributor Author

trivikr commented Nov 15, 2023

Are we not doing that anymore?

No. Removed in #2103

The aws-sdk is only in package.json https://github.com/search?q=repo%3Aemberjs%2Fember-inspector%20aws-sdk&type=code

@trivikr
Copy link
Contributor Author

trivikr commented Feb 19, 2024

@RobbieTheWagner Is there any other action needed to merge this PR?

I've rebase from main branch.

@RobbieTheWagner
Copy link
Member

@trivikr last I checked there were CI failures. If things pass this time, we can merge.

@RobbieTheWagner
Copy link
Member

Still getting some test failures in here

@trivikr
Copy link
Contributor Author

trivikr commented Feb 19, 2024

Two tests are failing

[test:ember] not ok 54 Chrome 121.0 - [513 ms] - Object Inspector: Date fields are editable
[test:ember]     ---
[test:ember]         actual: >
[test:ember]             0
[test:ember]         expected: >
[test:ember]             1
[test:ember]         stack: >
[test:ember]                 at Object.<anonymous> (http://localhost:7357/testing/assets/tests.js:69[97](https://github.com/emberjs/ember-inspector/actions/runs/7962892649/job/21741229328?pr=2511#step:9:98):18)
[test:ember]                 at callHook (http://localhost:7357/testing/assets/chunk.vendors-node_modules_pnpm_ember-sinon-qunit_7_4_0_ember-source_4_12_4_qunit_2_20_1_sinon_17_0-4350b3.d8ddfb3c2619f4ab8a81.js:877:806)
[test:ember]                 at runHook (http://localhost:7357/testing/assets/chunk.vendors-node_modules_pnpm_ember-sinon-qunit_7_4_0_ember-source_4_12_4_qunit_2_20_1_sinon_17_0-4350b3.d8ddfb3c2619f4ab8a81.js:886:1)
[test:ember]                 at ProcessingQueue.processTaskQueue (http://localhost:7357/testing/assets/chunk.vendors-node_modules_pnpm_ember-sinon-qunit_7_4_0_ember-source_4_12_4_qunit_2_20_1_sinon_17_0-4350b3.d8ddfb3c2619f4ab8a81.js:1133:121)
[test:ember]                 at ProcessingQueue.advanceTaskQueue (http://localhost:7357/testing/assets/chunk.vendors-node_modules_pnpm_ember-sinon-qunit_7_4_0_ember-source_4_12_4_qunit_2_20_1_sinon_17_0-4350b3.d8ddfb3c2619f4ab8a81.js:1127:136)
[test:ember]                 at ProcessingQueue.advance (http://localhost:7357/testing/assets/chunk.vendors-node_modules_pnpm_ember-sinon-qunit_7_4_0_ember-source_4_12_4_qunit_2_20_1_sinon_17_0-4350b3.d8ddfb3c2619f4ab8a81.js:1125:83)
[test:ember]         message: >
[test:ember]             The correct amount of objectInspector:saveProperty messages are sent
[test:ember]         negative: >
[test:ember]             false
[test:ember]         browser log: |
[test:ember]     ...
[test:ember] not ok 55 Chrome 121.0 - [397 ms] - Object Inspector: Errors are correctly displayed
[test:ember]     ---
[test:ember]         stack: >
[test:ember]                 at http://localhost:7357/testing/assets/chunk.vendors-node_modules_pnpm_ember-sinon-qunit_7_4_0_ember-source_4_12_4_qunit_2_20_1_sinon_17_0-4350b3.d8ddfb3c2619f4ab8a81.js:1729:251
[test:ember]         message: >
[test:ember]             global failure: Expecting 1 objectInspector:saveProperty messages, got 0
[test:ember]         negative: >
[test:ember]             false
[test:ember]         browser log: |
[test:ember]     ...

@trivikr
Copy link
Contributor Author

trivikr commented Feb 19, 2024

The failing tests:

  • Date fields are editable:
    test('Date fields are editable', async function (assert) {
    assert.expect(5);
    await visit('/');
    let date = new Date(2019, 7, 13); // 2019-08-13
    await sendMessage({
    type: 'objectInspector:updateObject',
    name: 'My Object',
    objectId: 'myObject',
    details: [
    {
    name: 'First Detail',
    expand: false,
    properties: [
    {
    name: 'dateProperty',
    value: {
    inspect: date.toString(),
    type: 'type-date',
    },
    },
    ],
    },
    ],
    });
    respondWith(
    'objectInspector:saveProperty',
    ({ objectId, property, value }) => {
    assert.strictEqual(typeof value, 'number', 'sent as timestamp');
    date = new Date(value);
    return {
    type: 'objectInspector:updateProperty',
    objectId,
    property,
    mixinIndex: 0,
    value: {
    inspect: date.toString(),
    type: 'type-date',
    isCalculated: false,
    },
    };
    }
    );
    await click('[data-test-object-detail-name]');
    assert.dom('[data-test-object-property-value]').hasText(date.toString());
    await click('[data-test-object-property-value]');
    let field = find('.js-object-property-value-date');
    assert.ok(field);
    respondWith(
    'objectInspector:saveProperty',
    ({ objectId, property, value }) => {
    assert.strictEqual(typeof value, 'number', 'sent as timestamp');
    date = new Date(value);
    return {
    type: 'objectInspector:updateProperty',
    objectId,
    property,
    mixinIndex: 0,
    value: {
    inspect: date.toString(),
    type: 'type-date',
    isCalculated: false,
    },
    };
    }
    );
    await fillIn(field, '2015-01-01');
    await triggerKeyEvent(field, 'keydown', 13);
    assert.dom('[data-test-object-property-value]').hasText(date.toString());
    });
  • Errors are correctly displayed:
    test('Errors are correctly displayed', async function (assert) {
    assert.expect(8);
    await visit('/');
    await sendMessage({
    type: 'objectInspector:updateObject',
    name: 'My Object',
    objectId: '1',
    details: [],
    errors: [{ property: 'foo' }, { property: 'bar' }],
    });
    assert.dom('[data-test-object-name]').hasText('My Object');
    assert.dom('[data-test-object-inspector-errors]').exists({ count: 1 });
    assert.dom('[data-test-object-inspector-error]').exists({ count: 2 });
    respondWith('objectInspector:traceErrors', ({ objectId }) => {
    assert.strictEqual(objectId, '1');
    return false;
    });
    await click('[data-test-send-errors-to-console]');
    await sendMessage({
    type: 'objectInspector:updateErrors',
    objectId: '1',
    errors: [{ property: 'foo' }],
    });
    assert.dom('[data-test-object-inspector-errors]').exists({ count: 1 });
    assert.dom('[data-test-object-inspector-error]').exists({ count: 1 });
    await sendMessage({
    type: 'objectInspector:updateErrors',
    objectId: '1',
    errors: [],
    });
    assert.dom('[data-test-object-inspector-errors]').doesNotExist();
    assert.dom('[data-test-object-inspector-error]').doesNotExist();
    });

I'm not sure why removal of unused aws-sdk should fail these tests.

@patricklx
Copy link
Collaborator

patricklx commented Feb 19, 2024

@trivikr the lock file shows many more updates done to the packages.
I'm not sure how you can reduce that to only 1 specific package.

Maybe if you only do pnpm uninstall <package> ?

@trivikr
Copy link
Contributor Author

trivikr commented Feb 20, 2024

Maybe if you only do pnpm uninstall ?

I used pnpm uninstall to remove the dependency, and force pushed.

Can you restart CI?

@trivikr
Copy link
Contributor Author

trivikr commented Feb 20, 2024

The CI is successful. Is this good to merge?

@RobbieTheWagner RobbieTheWagner merged commit 56575c0 into emberjs:main Feb 20, 2024
15 checks passed
@github-actions github-actions bot mentioned this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants